Oh, gahk... I should know better than to post a patch from my library w/o checking it over...
As originally posted, it doesn't allow NO url (http://xxx:3000/) and ones without an extension... Corrected patch: ===========================B E G I N====================== --- ntop-original/http.c Tue Dec 18 17:05:08 2001 +++ ntop-current/http.c Fri Dec 21 15:42:06 2001 @@ -780,17 +780,84 @@ static int checkURLsecurity(char *url) { - int rc = 0, i, len=strlen(url); + int rc = 0, i, extCharacter, countOKext, len=strlen(url); + char ext[4]; - for(i=1; i<len; i++) - if((url[i] == '.') && (url[i-1] == '.')) { - rc = 1; - break; - } else if((url[i] == '/') && (url[i-1] == '/')) { - rc = 1; - break; - } else if((url[i] == '.') && (url[i-1] == '/')) { - rc = 1; - break; - } +/* BMS 12-2001 + Let's be really smart about this - instead of defending against + hostile requests we don't yet know about, let's make sure it + we only serve up the very limited set of pages we're interested + in serving up... + + http://server[:port]/url + Our urls end in .htm(l), .css, .jpg, .gif or .png + + We don't want to serve requests that attempt to hide or obscure our + server. Yes, we MIGHT somehow reject a marginally legal request, but + tough! + + Any character that shouldn't be in a CLEAR request, causes us to + bounce the request... + + For example, + //, .. and /. -- directory transversal + \r, \n -- used to hide stuff in logs + :, @ -- used to obscure logins, etc. + unicode exploits -- used to hide the above + + */ + + /* No URL? That is our default action... */ + if (len == 0) { + return(0); + } + + /* Unicode encoded, a : or @ or \r or \n - no dice */ + if (strcspn(url, "%:@\r\n") < len) { + traceEvent(TRACE_ERROR, "Found % : @ \\r or \\n in URL...\n"); + return(1); + } + + /* a double slash? */ + if (strstr(url, "//") > 0) { + traceEvent(TRACE_ERROR, "Found // in URL...\n"); + return(1); + } + + /* a double dot? */ + if (strstr(url, "..") > 0) { + traceEvent(TRACE_ERROR, "Found .. in URL...\n"); + return(1); + } + + /* let's check after the each . (page extension), so + x.gif.vbs doesn't get past us. Since there should + only be ONE extension, we'll simply and reject if + ANY extension is bad. */ + countOKext = 0; + for (i=0; i<len; i++) { + if (url[i] == '.') { + for (extCharacter=1; extCharacter<=3; extCharacter++) { + ext[extCharacter-1] = (i+extCharacter < len) ? (char) tolower(url[i+extCharacter]) : '\0'; + } + ext[3] = '\0'; + + if ( (strcmp(ext , "htm") == 0) || + (strcmp(ext , "jpg") == 0) || + (strcmp(ext , "png") == 0) || + (strcmp(ext , "gif") == 0) || + (strcmp(ext , "css") == 0) ) { + countOKext++; + continue; + } else { + /* bad extension! */ + rc=1; + break; + } + } + } + + if (countOKext > 1) { + rc=1; + } return(rc); =============================E N D======================== -----Original Message----- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Burton M. Strauss III Sent: Friday, December 21, 2001 3:03 PM To: Ntop-Dev Cc: Luca Deri Subject: [Ntop-dev] [PATCH-improvement] http.c - checkURLsecurity In reading bugtraq, et al, I've noticed that nothing is "safe" from being exploited. I noticed that checkURLsecurity in http.c wasn't secure against Unicode exploits. So I've made the following change: Comments welcome! -----Burton <snip> _______________________________________________ Ntop-dev mailing list [EMAIL PROTECTED] http://listmanager.unipi.it/mailman/listinfo/ntop-dev
