255118 - xpcom glue sprintf can overflow buffer

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Daniel Veditz [:dveditz]]

in nsGREDirServiceProvider.cpp the GRE path is gotten from the environment
variable and sprintf'd into a MAXPATHLEN buffer. Use snprintf() to make sure
malicious stuff doesn't crash us.

Comment 1

[Doug Turner (:dougt)]

Index: nsGREDirServiceProvider.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/glue/standalone/nsGREDirServiceProvider.cpp,v
retrieving revision 1.28
diff -u -r1.28 nsGREDirServiceProvider.cpp
--- nsGREDirServiceProvider.cpp	2 Apr 2004 09:16:36 -0000	1.28
+++ nsGREDirServiceProvider.cpp	11 Aug 2004 15:06:50 -0000
@@ -501,7 +501,7 @@
     }
   }
 
-  sprintf(sXPCOMPath, "%s" XPCOM_FILE_PATH_SEPARATOR XPCOM_DLL, grePath);
+  PR_snprintf(sXPCOMPath, MAXPATHLEN, "%s" XPCOM_FILE_PATH_SEPARATOR XPCOM_DLL,
grePath);
 
   return sXPCOMPath;
 }

Comment 2

[Daniel Veditz [:dveditz]]

r/sr=dveditz for trivial fix.

Comment 3

[Benjamin Smedberg]

I was trying really hard at one point to rid the XPCOM glue of all dependencies
on NSPR, so that we don't have the weird issues with apps shipping NSPR and
having incompatible NSPR versions. This was never finished, but I think I have a
tree lying around which does all the platform-specific linking stuff. Maybe it's
not important in a libxul world?

Comment 4

[Doug Turner (:dougt)]

ben, look 6 lines above this change -- we are using PR_GetEnv.  A simple search
through this file will reveal 10 nspr calls.  Adding another isn't going to hurt
anything.  If you want to use something else, feel free to post a patch.  I will
wait on checking this in for a day or two. 

Comment 5

[Daniel Veditz [:dveditz]]

If you do get rid of the other nspr calls this location could be easily fixed
using a format specifier, if you don't mind hardcoding buffer length minus the
fixed part of the strings (could go out of date since the others are #defines),
or calculating strlen() as an equivalent spot in the gre directory service does.

Comment 6

[Tom Tromey :tromey]

I was looking at printf-related bugs and noticed that this one is obsolete.
The file in question no longer exists, nor does sXPCOMPath.
So, I'm closing it.