Author: rmeggins
Update of /cvs/dirsec/adminserver/lib/base
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv28761/adminserver/lib/base
Modified Files:
util.cpp
Log Message:
Bug(s) fixed: 186280
Bug Description: adminserver: Close potential security vulnerabilities
in CGI code
Reviewed by: Rob, Pete, Nathan, Noriko (Thanks!)
Fix Description: Most of this just involves making sure that we use
PR_snprintf/PL_strncpyz/PL_strcatn where able, or just making sure we
use snprintf/strncpy/strncat correctly and null terminate the buffers.
I also got rid of some dead code, unused variables, and the like. There
are a few cases that are more complex that I have specified below. In
some cases I had to change the function signature to add a size
parameter in cases where the function was copying to a given char * and
the size was assumed (in most cases this was safe but it's still dangerous).
Platforms tested: Fedora Core 5
Flag Day: no
Doc impact: no
Index: util.cpp
===================================================================
RCS file: /cvs/dirsec/adminserver/lib/base/util.cpp,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- util.cpp 18 Aug 2005 19:18:27 -0000 1.4
+++ util.cpp 31 Mar 2006 22:58:22 -0000 1.5
@@ -327,7 +327,9 @@
/* Standard HTTP (RFC 850) starts with dd-mon-yy */
if(ims[2] == '-') {
- sscanf(ims, "%s %d:%d:%d", t, &h, &m, &s);
+ /* Warning - hardcoded 128 is sizeof(t) - scanf is not security conscious */
+ sscanf(ims, "%128s %d:%d:%d", t, &h, &m, &s);
+ t[sizeof(t)-1] = 0;
if(strlen(t) < 6)
return 0;
t[2] = '\0';
@@ -340,12 +342,16 @@
}
/* The ctime format starts with a month name */
else if(isalpha(*ims)) {
- sscanf(ims,"%s %d %d:%d:%d %*s %d", t, &d, &h, &m, &s,
&y);
+ /* Warning - hardcoded 128 is sizeof(t) - scanf is not security conscious */
+ sscanf(ims,"%128s %d %d:%d:%d %*s %d", t, &d, &h, &m,
&s, &y);
+ t[sizeof(t)-1] = 0;
mnum = _mstr2num(t);
}
/* RFC 822 */
else {
- sscanf(ims, "%d %s %d %d:%d:%d", &d, t, &y, &h, &m,
&s);
+ /* Warning - hardcoded 128 is sizeof(t) - scanf is not security conscious */
+ sscanf(ims, "%d %128s %d %d:%d:%d", &d, t, &y, &h, &m,
&s);
+ t[sizeof(t)-1] = 0;
mnum = _mstr2num(t);
}