pdate of /cvsroot/common/util
In directory cvs01.internal.helixcommunity.org:/tmp/cvs-serv20623
Modified Files:
Tag: hxclient_1_2_1_neptune
hxurl.cpp
Log Message:
Ref: https://bugs.dev.prognet.com/show_bug.cgi?id=Index:
There is a bug (probably several) in our old 121Nep branch
where we do not parse URLs correctly. The one below is the
cause of the above security bug. Modern branches have, what
amounts to, a complete re-write of our parsing code. I don't
feel taking all of those changes back into 121Nep can be
justified due to the QA hit and the timing of this release.
There would be lots of changes needed. So, I just fixed this
particular bug.
This bug can be exercised by this simple code:
main()
{
char *pp = new char[123];
strcpy( pp,"http://AAA.BBB.CCC.DDD:EEEE/%.20000000s%" );
CHXURL::Unescape(pp);
}
The problem was that the code assumed there were always 2 digits
after the '%' (excape symbol). One wrinkle though. I can not
build this branch locally anymore, my compiler is too new. The
below diff *does* fix the crash however, I just won't know what
happens up the rest of the call chain. I need to check this in
and then build on the farm to see if there are any other problems
that come into play (security/crash related only).
This will be for SlipStream 18 (121Nep) only.
Index: hxurl.cpp
===================================================================
RCS file: /cvsroot/common/util/hxurl.cpp,v
retrieving revision 1.24.4.1
retrieving revision 1.24.4.1.4.1
diff -u -d -r1.24.4.1 -r1.24.4.1.4.1
--- hxurl.cpp 4 Feb 2004 02:15:16 -0000 1.24.4.1
+++ hxurl.cpp 3 Jul 2007 17:12:41 -0000 1.24.4.1.4.1
@@ -1428,15 +1428,24 @@
{
if ( (!bProcessingOptionsPastQuestionMark) && (*s == '%') )
{
- if (*++s != '\0')
- {
- *p = Unhex( *s ) << 4;
- }
-
- if (*++s != '\0')
- {
- *p++ += Unhex( *s );
- }
+ //There needs to be at least 2 more chars
+ if( strlen(s) > 2 )
+ {
+ if( *++s != '\0' )
+ {
+ *p = Unhex( *s ) << 4;
+
+ if( *++s != '\0' )
+ {
+ *p++ += Unhex( *s );
+ }
+ }
+ }
+ else
+ {
+ //invalid escaped URL.
+ break;
+ }
}
else
{
