tags 475733 + patch tags 476603 + patch thanks Hi, attached is a patch for acon which I can't test. Since this involves quite a few changes it would be nice if someone could review and/or test this patch.
Kind regards Nico -- Nico Golde - http://www.ngolde.de - [EMAIL PROTECTED] - GPG: 0x73647CFF For security reasons, all text in this mail is double-rot13 encrypted.
diff -u acon-1.0.5/debian/changelog acon-1.0.5/debian/changelog
--- acon-1.0.5/debian/changelog
+++ acon-1.0.5/debian/changelog
@@ -1,3 +1,11 @@
+acon (1.0.5-6.1) unstable; urgency=high
+
+ * Non-maintainer upload by the Security Team.
+ * Fix various buffer overflows by doing proper bounds checking
+ that could be exploited to get root access (CVE-2008-1994; Closes: #476603).
+
+ -- Nico Golde <[EMAIL PROTECTED]> Sat, 24 May 2008 22:10:40 +0200
+
acon (1.0.5-6) unstable; urgency=low
* Added doc/readme* to docs.
diff -u acon-1.0.5/debian/patches/04_loadkeys.diff acon-1.0.5/debian/patches/04_loadkeys.diff
--- acon-1.0.5/debian/patches/04_loadkeys.diff
+++ acon-1.0.5/debian/patches/04_loadkeys.diff
@@ -12,11 +12,11 @@
{
+ // aelmahmoudy: removed ">& /dev/null" from string, as it is not needed:
if(path[0]!='/')
-- sprintf(tmp,"loadkeys %s/keymaps/%s >& /dev/null",DATAPATH,path);
-+ sprintf(tmp,"loadkeys %s/keymaps/%s",DATAPATH,path);
+- snprintf(tmp, sizeof(tmp),"loadkeys %s/keymaps/%s >& /dev/null",DATAPATH,path);
++ snprintf(tmp, sizeof(tmp),"loadkeys %s/keymaps/%s",DATAPATH,path);
else
-- sprintf(tmp,"loadkeys %s >& /dev/null",path);
-+ sprintf(tmp,"loadkeys %s",path);
+- snprintf(tmp, sizeof(tmp),"loadkeys %s >& /dev/null",path);
++ snprintf(tmp, sizeof(tmp),"loadkeys %s",path);
}
else
sprintf(tmp,"loadkeys %s/keymaps/iso8859-6.map",DATAPATH);
only in patch2:
unchanged:
--- acon-1.0.5.orig/menu.c
+++ acon-1.0.5/menu.c
@@ -20,6 +20,7 @@
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
+#include <limits.h>
#include <string.h>
#include <linux/vt.h>
#include <dirent.h>
@@ -48,8 +49,9 @@
int i;
memset(line,0,400);
- for(i=0;i<getmaxy();i++)
+ for(i=0;i<getmaxy() && i < sizeof(line) - 1;i++)
putlinexy(vcsa,0,i,line,getmaxx());
+ line[sizeof(line) - 1] = 0;
}
int drawmenuxy(int vcsa,int x,int y,int xwidth,int ywidth,char **menu,int num)
@@ -65,7 +67,7 @@
{
line[0]=0x86;
line[1]=COLORN;
- for(i=2;i<xwidth*2+2;i+=2)
+ for(i=2;i<xwidth*2+2 && i < sizeof(line) - 3;i+=2)
line[i]=0x81,line[i+1]=COLORN;
line[i]=0x85;
line[i+1]=COLORN;
@@ -77,7 +79,7 @@
{
if(i+starty<num)
- for(z=0;z<xwidth*2;z+=2)
+ for(z=0;z<xwidth*2 && z < sizeof(line) - 4;z+=2)
{
if(z/2<strlen(menu[i+starty]))
line[z+2]=menu[i+starty][z/2];
@@ -86,7 +88,7 @@
line[z+3]=(i+starty==select?COLORS:COLORN);
}
else
- for(z=0;z<xwidth*2;z+=2)
+ for(z=0;z<xwidth*2 && sizeof(line) - 4 ;z+=2)
{
line[z+2]=' ';
line[z+3]=(i+starty==select?COLORS:COLORN);
@@ -96,7 +98,7 @@
}
line[0]=0x84;
line[1]=COLORN;
- for(i=2;i<xwidth*2+2;i+=2)
+ for(i=2;i<xwidth*2+2 && i < sizeof(line) - 3;i+=2)
line[i]=0x81,line[i+1]=COLORN;
line[i]=0x83;
line[i+1]=COLORN;
@@ -150,7 +152,7 @@
len=0;
if((dp=opendir(path))==NULL)
return NULL;
- while((ep=readdir(dp)))
+ while((ep=readdir(dp)) && len < (UINT_MAX - 1) / sizeof(char *))
if(ep->d_type!=DT_DIR && ep->d_name[0]!='.')len++;
closedir(dp);
if(len==0)return NULL;
@@ -217,31 +219,33 @@
line[0]=0x86;
line[1]=COLORN;
- for(i=2;i<xwidth*2-2;i+=2)
+ for(i=2;i<xwidth*2-2 && i < sizeof(line) - 3;i+=2)
line[i]=0x81,line[i+1]=COLORN;
line[i]=0x85;
line[i+1]=COLORN;
putlinexy(vcsa,5,ypos,line,xwidth+2);
line[0]=0x84;
- for(i=2;i<xwidth*2-2;i+=2)
+ for(i=2;i<xwidth*2-2 && i < sizeof(line) - 2;i+=2)
line[i]=0x81;
line[i]=0x83;
putlinexy(vcsa,5,ypos+2,line,xwidth+2);
line[0]=0x80;
line[i]=0x80;
- for (i=0;i<strlen(p)*2;i+=2)
+ for (i=0;i<strlen(p)*2 && i < sizeof(line) - 4;i+=2)
line[i+2]=p[i/2];
startpos=i+2;
- for(i=startpos;i<xwidth*2-2;i+=2)
+ for(i=startpos;i<xwidth*2-2 && i < sizeof(line) - 3;i+=2)
line[i+1]=COLORE;
while(1)
{
- for(i=startpos;i<xwidth*2-2;i+=2)
+ for(i=startpos;i<xwidth*2-2 && i < sizeof(line) - 2;i+=2)
line[i]=' ';
- for(i=0;i<strlen(str);i++)
+ /* integer overflow but this would only result in overwriting already
+ * written buffer content, no security issue */
+ for(i=0;i<strlen(str) && i * 2 + startpos < sizeof(line);i++)
line[i*2+startpos]=str[i];
putlinexy(vcsa,5,ypos+1,line,xwidth+2);
gotoxy(vcsa,5+startpos/2+strlen(str),ypos+1);
only in patch2:
unchanged:
--- acon-1.0.5.orig/arabicfont.c
+++ acon-1.0.5/arabicfont.c
@@ -587,7 +587,8 @@
setfont(tty,fontwidth,font);
return;
}
- strcpy(oldpath,path);
+ strncpy(oldpath,path, sizeof(oldpath)-1);
+ oldpath[sizeof(oldpath)-1]=0;
}
if(!path)
{
@@ -613,9 +614,11 @@
y=16; /*Only support 8x16 fonts now*/
if(path[0]!='/')
- sprintf(tmp,"%s/fonts/%s",DATAPATH,path);
- else
- strcpy(tmp,path);
+ snprintf(tmp, sizeof(tmp), "%s/fonts/%s",DATAPATH,path);
+ else{
+ strncpy(tmp,path, sizeof(tmp));
+ tmp[sizeof(tmp)-1]=0;
+ }
set_user_id();
if((fp=fopen(tmp,"r"))==NULL)
{
@@ -687,7 +690,8 @@
{
if(!strcmp(path,oldpath))
return;
- strcpy(oldpath,path);
+ strncpy(oldpath,path, sizeof(oldpath)-1);
+ oldpath[sizeof(oldpath)-1]=0;
}
if(!needrefreshconsole && !path)
{
@@ -699,9 +703,9 @@
if(path)
{
if(path[0]!='/')
- sprintf(tmp,"loadkeys %s/keymaps/%s >& /dev/null",DATAPATH,path);
+ snprintf(tmp, sizeof(tmp),"loadkeys %s/keymaps/%s >& /dev/null",DATAPATH,path);
else
- sprintf(tmp,"loadkeys %s >& /dev/null",path);
+ snprintf(tmp, sizeof(tmp),"loadkeys %s >& /dev/null",path);
}
else
sprintf(tmp,"loadkeys %s/keymaps/iso8859-6.map",DATAPATH);
@@ -723,7 +727,8 @@
{
if(!strcmp(path,oldpath))
return;
- strcpy(oldpath,path);
+ strncpy(oldpath,path, sizeof(oldpath)-1);
+ oldpath[sizeof(oldpath)-1]=0;
}
if(!needrefreshconsole && !path)
{
@@ -743,7 +748,7 @@
}
if(path[0]!='/')
- sprintf(tmp,"%s/translations/%s",DATAPATH,path);
+ snprintf(tmp,sizeof(tmp),"%s/translations/%s",DATAPATH,path);
else
strcpy(tmp,path);
if((fp=fopen(tmp,"r"))==NULL)
@@ -761,7 +766,7 @@
while(!feof(fp))
{
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
/*Ignore comments*/
@@ -775,7 +780,7 @@
i=0;
do
{
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
/*Ignore comments*/
@@ -799,7 +804,7 @@
i=0;
do
{
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
/*Ignore comments*/
@@ -825,7 +830,7 @@
i=0;
do
{
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
/*Ignore comments*/
@@ -850,7 +855,7 @@
i=0;
do
{
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
/*Ignore comments*/
@@ -875,7 +880,7 @@
i=0;
do
{
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
/*Ignore comments*/
only in patch2:
unchanged:
--- acon-1.0.5.orig/acon.c
+++ acon-1.0.5/acon.c
@@ -50,7 +50,7 @@
font[0]=translation[0]=keymap[0]=0;
if((env=getenv("HOME")))
- sprintf(tmp,"%s/.acon.conf",env);
+ snprintf(tmp, sizeof(tmp), "%s/.acon.conf",env);
else
strcpy(tmp,"/etc/acon.conf");
if((fp=fopen(tmp,"r"))==NULL)
@@ -62,7 +62,7 @@
while(!feof(fp))
{
char *s;
- fgets(tmp,300,fp);
+ fgets(tmp,sizeof(tmp),fp);
s=tmp;
if(strchr(s,'\n'))*strchr(s,'\n')=0;
@@ -136,7 +136,8 @@
i=strlen(s)-1;
while(s[i] && isspace(s[i]) && i>0)i--;
s[i+1]=0;
- strcpy(font,s);
+ strncpy(font,s, sizeof(font)-1);
+ font[sizeof(font) -1]=0;
}
else if(!strncmp(s,"keymap",6))
{
@@ -148,7 +149,8 @@
i=strlen(s)-1;
while(s[i] && isspace(s[i]) && i>0)i--;
s[i+1]=0;
- strcpy(keymap,s);
+ strncpy(keymap,s,sizeof(keymap)-1);
+ keymap[sizeof(keymap)-1]=0;
}
else if(!strncmp(s,"translation",11))
{
@@ -160,7 +162,8 @@
i=strlen(s)-1;
while(s[i] && isspace(s[i]) && i>0)i--;
s[i+1]=0;
- strcpy(translation,s);
+ strncpy(translation,s,sizeof(translation)-1);
+ translation[sizeof(translation)-1]=0;
}
else
printf("Syntax error in config file '%s'\n",tmp);
only in patch2:
unchanged:
--- acon-1.0.5.orig/child.c
+++ acon-1.0.5/child.c
@@ -67,6 +67,7 @@
/*Add new console to the list of the consoles to be monitored*/
int addconsole(int cnum)
{
+ if(cnum < 0) return -3;
if(isinlist(cnum))return -1;
if(consolesn==63)return -2;
memset(&condata[consolesn],0,sizeof(*condata));
@@ -78,6 +79,7 @@
void removeconsole(int cnum)
{
int i;
+ if(cnum < 0) return;
setactive(cnum);
cnum=getstructnum(cnum);
for(i=cnum;i<consolesn-1;i++)
@@ -101,7 +103,7 @@
printf("Acon: can't know HOME directory\n");
return;
}
- sprintf(tmp,"%s/.acon.conf",env);
+ snprintf(tmp, sizeof(tmp),"%s/.acon.conf",env);
if((fp=fopen(tmp,"w"))==NULL)
{
printf("Acon: can't save %s\n",tmp);
only in patch2:
unchanged:
--- acon-1.0.5.orig/render.c
+++ acon-1.0.5/render.c
@@ -116,14 +116,15 @@
unsigned char curloc[200];
int lang=0; /*0=english 1=arabic*/
- for(i=0;i<len;i+=2)
+ /* if i < sizeof(curldoc) => i < sizeof(newline), they are of the same size */
+ for(i=0;i<len && i < sizeof(curloc);i+=2)
{
newline[i/2]=isotocp(line[i]);
newlinegroub[i/2]=groub;
curloc[i]=200;
}
- for(i=0;i<len;i+=2)
+ for(i=0;i<len && i < sizeof(newline);i+=2)
{
groub=tmp=newlinegroub[i/2];
if(change>i){
@@ -189,9 +190,10 @@
locn+=2;
}
}
- memcpy(line,buf,len);
+ if(len > 0)
+ memcpy(line,buf,len);
if(scrn.y==ypos)
- for(i=len/2;i>=0;i--)
+ for(i=len/2;i>=0 && i < sizeof(curloc);i--)
if(curloc[i]==scrn.x)
{scrn.x=i;break;}
@@ -204,7 +206,7 @@
char curloc[200];
int lang=1; /*0=english 1=arabic*/
- for(i=0;i<len;i+=2)
+ for(i=0;i<len && i < sizeof(curloc);i+=2)
{
newline[i/2]=isotocp(line[i]);
newlinegroub[i/2]=groub;
@@ -260,7 +262,8 @@
buf[len-loc+1-2]=line[i+1];
}
}
- memcpy(line,buf,len);
+ if(len > 0)
+ memcpy(line,buf,len);
if(scrn.y==ypos)
for(i=len/2;i>=0;i--)
@@ -342,7 +345,7 @@
printf("lines %d cols %d \n",(int)scrn.lines,(int)scrn.cols);
#endif
- for(i=0;i<scrn.lines;i++)
+ for(i=0;i<scrn.lines && i < sizeof(line) - 2;i++)
{
ypos=i;
if(read (consolevc,line+2,((size_t)scrn.cols)*2)!=scrn.cols*2){
pgp4hWX6OKRmh.pgp
Description: PGP signature

