Control: tags -1 + patch On 2013-02-05 11:01:39, Sang Kil Cha wrote: > imview has stack smashing vulnerability when parsing ics header @ > io/readics.cxx:320 > > /* get the filename from the ICS file */ > > t = temp1; > while (*bp != delim2) > *t++ = *bp++;
The attached patch should fix this bug. It adds bounds checking for all the parts that read in a way like that. The provided file doesn't crash with the patch, but since I don't have any ICS images, someone else should check if the patch doesn't break anything. Regards -- Sebastian Ramacher
--- a/io/readics.cxx
+++ b/io/readics.cxx
@@ -273,6 +273,13 @@
/* get the length of the ICS file and rewind */
length = (unsigned int)lseek(fd,0L,2);
lseek(fd,0L,0);
+
+ /* the first two characters are the seperators */
+ if (length < 2)
+ {
+ close(fd);
+ return -4;
+ }
/* allocate space for all data from the ICS file */
if ((buffer1 = (char *)malloc(length)) == NULL)
@@ -321,10 +328,15 @@
delim1 = *bp++; /* field delimiter */
delim2 = *bp++; /* record delimiter */
t = temp1;
-
+
+ size_t bread = 0;
+
/* check if written by ICS */
- while (*bp != delim2)
- *t++ = *bp++;
+ while (*bp != delim2 && bread < 3 && bp != end)
+ {
+ *t++ = *bp++;
+ ++bread;
+ }
bp++;
*t = '\0';
if (strncmp(temp1,"ICS",3) && strncmp(temp1,"ics",3))
@@ -337,13 +349,18 @@
/* get the filename from the ICS file */
t = temp1;
- while (*bp != delim2)
- *t++ = *bp++;
+ bread = 0;
+ while (*bp != delim2 && bread < sizeof(temp1) - 1 && bp != end)
+ {
+ *t++ = *bp++;
+ ++bread;
+ }
bp++;
*t = '\0';
t = strchr(temp1,delim1);
- strcpy(icsheader->filename,t);
+ strncpy(icsheader->filename,t, FILENAME_SIZE);
+ icsheader->filename[FILENAME_SIZE - 1] = '\0';
*t = '\0';
if (strcmp(temp1,"filename"))
@@ -360,18 +377,27 @@
{
/* get the next record into temp1 */
t = temp1;
- while (*bp != delim2 && bp < end) /* dont read beyond EOF */
- *t++ = *bp++;
+ bread = 0;
+ while (*bp != delim2 && bp < end && bread < sizeof(temp1) - 1) /* dont read beyond EOF */
+ {
+ *t++ = *bp++;
+ ++bread;
+ }
bp++;
*t = '\0';
/* get the category into temp2 */
+ bread = 0;
t = temp1;
tg = temp2;
- while (*t != delim1)
+ while (*t != delim1 && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
t++;
*tg = '\0';
+ ++bread;
/* check if it is one of the decodable categories */
cat = 0;
@@ -388,10 +414,14 @@
}
/* get the next field from this record */
tg = temp2;
- while (*t != delim1)
+ while (*t != delim1 && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
t++;
*tg = '\0';
+ ++bread;
/* find this item in the keyword table */
for (i = 0; i < kwrds; i++)
@@ -415,10 +445,14 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
icsheader->parameters = atoi(temp2);
if (icsheader->parameters > MAXDIM)
{ /* if necessary change MAXDIM in ics.h */
@@ -444,11 +478,15 @@
for (i = 0; i < icsheader->parameters; i++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->order[i],temp2);
+ strncpy(icsheader->order[i],temp2, ORDER_SIZE);
+ icsheader->order[i][ORDER_SIZE - 1] = '\0';
}
icsheader->valid_order = TRUE;
break;
@@ -468,10 +506,14 @@
for (i = 0; i < icsheader->parameters; i++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
icsheader->sizes[i] = atoi(temp2);
}
icsheader->valid_sizes = TRUE;
@@ -484,11 +526,16 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->coord,temp2);
+ ++bread;
+ strncpy(icsheader->coord,temp2, COORD_SIZE);
+ icsheader->coord[COORD_SIZE - 1] = '\0';
icsheader->valid_coord = TRUE;
break;
case 4: /* significant bits */
@@ -499,10 +546,14 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
icsheader->sigbits = atoi(temp2);
icsheader->valid_sigbits = TRUE;
break;
@@ -514,11 +565,16 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->format,temp2);
+ ++bread;
+ strncpy(icsheader->format,temp2, FORMAT_SIZE);
+ icsheader->format[FORMAT_SIZE - 1] = '\0';
icsheader->valid_format = TRUE;
break;
case 6: /* signed or unsigned */
@@ -529,10 +585,14 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
if (!strcmp(temp2,"unsigned"))
icsheader->sign = UNSIGNED;
else icsheader->sign = SIGNED;
@@ -546,11 +606,16 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->compression,temp2);
+ ++bread;
+ strncpy(icsheader->compression,temp2, CMPS_SIZE);
+ icsheader->compression[CMPS_SIZE - 1] = '\0';
icsheader->valid_compression = TRUE;
break;
case 8: /* origin */
@@ -569,10 +634,14 @@
for (i = 0; i < icsheader->parameters; i++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
icsheader->origin[i] = (float)atof(temp2);
}
icsheader->valid_origin = TRUE;
@@ -593,10 +662,14 @@
for (i = 0; i < icsheader->parameters; i++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
icsheader->scale[i] = (float)atof(temp2);
}
icsheader->valid_scale = TRUE;
@@ -617,11 +690,16 @@
for (i = 0; i < icsheader->parameters; i++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->label[i],temp2);
+ ++bread;
+ strncpy(icsheader->label[i],temp2, LABEL_SIZE);
+ icsheader->label[i][LABEL_SIZE - 1] = '\0';
}
icsheader->valid_label = TRUE;
break;
@@ -641,11 +719,16 @@
for (i = 0; i < icsheader->parameters; i++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->units[i],temp2);
+ ++bread;
+ strncpy(icsheader->units[i],temp2, UNITS_SIZE);
+ icsheader->units[i][UNITS_SIZE - 1] = '\0';
}
icsheader->valid_units = TRUE;
break;
@@ -666,10 +749,14 @@
for (ui = 0; ui < length; ui++)
{
tg = temp2;
- while (*t != delim1 && *t != '\0')
+ while (*t != delim1 && *t != '\0' && bread < sizeof(temp1) - 1)
+ {
*tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
+ ++bread;
icsheader->byteorder[ui] = atoi(temp2);
}
icsheader->valid_byteorder = TRUE;
@@ -682,11 +769,16 @@
break;
}
tg = temp2;
- while (*t != '\0')
- *tg++ = *t++;
+ while (*t != '\0' && bread < sizeof(temp1) - 1)
+ {
+ *tg++ = *t++;
+ ++bread;
+ }
*tg = '\0';
t++;
- strcpy(icsheader->SCIL_TYPE,temp2);
+ ++bread;
+ strncpy(icsheader->SCIL_TYPE,temp2, SCIL_SIZE);
+ icsheader->SCIL_TYPE[SCIL_SIZE - 1] = '\0';
icsheader->valid_SCIL_TYPE = TRUE;
break;
default:
signature.asc
Description: Digital signature
