On 2008-10-16 08:21, Jan Flokstra wrote:

Update of /cvsroot/monetdb/pathfinder/runtime In directory 23jxhf1.ch3.sourceforge.com:/tmp/cvs-serv11903

Modified Files: shredder.mx Log Message:

• fix strncat() size overflow compilation problem on some architectures.

U shredder.mx Index: shredder.mx =================================================================== RCS file: /cvsroot/monetdb/pathfinder/runtime/shredder.mx,v retrieving revision 1.142 retrieving revision 1.143 diff -u -d -r1.142 -r1.143 --- shredder.mx 15 Oct 2008 12:45:12 -0000 1.142 +++ shredder.mx 16 Oct 2008 06:21:44 -0000 1.143 @@ -1324,7 +1324,7 @@

const xmlChar *abs_SystemID;

• char locbuff[FILENAME_MAX];

• char locbuff[FILENAME_MAX+1];

  if (SystemID && shredCtx->base_dir && !isURL((char*)SystemID) && !((*SystemID=='/') || (*SystemID=='\'))) { /* is is a relative name which should be prefixed with the

@@ -1333,7 +1333,7 @@ /* base_dir */ locbuff[0] = '\0'; strncat(&locbuff[0],shredCtx->base_dir,FILENAME_MAX);

•    strncat(&locbuff[0],(char*)SystemID,FILENAME_MAX);
  

•    strncat(&locbuff[0],(char*)SystemID,FILENAME_MAX-strlen(shredCtx->base_dir));
   abs_SystemID = (const xmlChar *)&locbuff[0];
  

  } else abs_SystemID = SystemID;

This is not very good from a defensive programming point of view, nor from an efficiency point of view. Plus, the compiler complains about the second strncat. Better, instead of the three lines starting with locbuff[0] = '\0'; do snprintf(locbuff, sizeof(locbuff), "%s%s", shredCtx->base_dir, (char *) SystemID);

The problem is, strlen(shredCtx->base_dir) could be greater than FILENAME_MAX, in which case the third argument to the second strncat could overflow and become extremely large (it's an unsigned, so it won't be negative).