chesterpolo / mongoose

Automatically exported from code.google.com/p/mongoose
MIT License
0 stars 0 forks source link

Directory Traversal Security Vulnerability #90

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
It is possible to bypass the ".." check in fix_directory_separators() by
using "%c2.%c2" which will be converted to ".." after the call to
MultiByteToWideChar(). The filtering check should be moved after the
conversion.

Example request:

http://localhost:8080/%c2.%c2./%c2.%c2./%c2.%c2./%c2.%c2./%c2.%c2./%c2.%c2./etc/
passwd

This bug was NOT found by me but rather by Gera from CORE Security
(http://www.coresecurity.com) during a contest I hosted recently at a
security conference. We planted two bugs within the mongoose source and
Gera found this one while looking for them. If you're curious about the
contest some details are here:

http://dvlabs.tippingpoint.com/blog/2009/09/08/ekoparty-2009

Original issue reported on code.google.com by pedram.amini on 21 Sep 2009 at 1:39

GoogleCodeExporter commented 9 years ago
I was running it on wine when I tested it, that's why you see etc/passwd in the 
URL :)

Original comment by g...@corest.com on 22 Sep 2009 at 1:58

GoogleCodeExporter commented 9 years ago
Thanks guys.
To be fixed. I assume you've tried to crack UNIX code path, too?
If you have any suspicions, I'd love to hear. And, any advice about techniques &
tools you're using.

Original comment by valenok on 27 Sep 2009 at 10:10

GoogleCodeExporter commented 9 years ago
Hi!
I don't know if you fixed this, but here 
( http://www.milw0rm.com/exploits/8428 )
seems hackers just detected this bug some months ago.

Bye!

Original comment by marcu...@gmail.com on 6 Oct 2009 at 6:41

GoogleCodeExporter commented 9 years ago
i haven't seen this major issue fixed. has the dev of this very promising web 
server
stopped?

Original comment by ac4...@gmail.com on 18 Jan 2010 at 2:14

GoogleCodeExporter commented 9 years ago
Not stopped, but is going slowly.
When I manage to get some spare cycles, I work on core and API enhancements I 
announced recently. This bug is fairly important, but not top priority for me, 
and will be 
fixed after the core has been refactored.

Original comment by valenok on 25 Feb 2010 at 12:56

GoogleCodeExporter commented 9 years ago
thanks - when do you expect the core to be re factored?

Original comment by mous...@gmail.com on 25 Feb 2010 at 11:52

GoogleCodeExporter commented 9 years ago
Well this is hard to answer. O(month) if everything works as expected.

Original comment by valenok on 25 Feb 2010 at 12:27

GoogleCodeExporter commented 9 years ago
You can fix this by adding 3 lines to the bottom of the to_unicode method:

static void
remove_double_dots_and_double_slashes(char *s);

/*
 * Encode 'path' which is assumed UTF-8 string, into UNICODE string.
 * wbuf and wbuf_len is a target buffer and its length.
 */
static void
to_unicode(const char *path, wchar_t *wbuf, size_t wbuf_len)
{
    char    buf[FILENAME_MAX], *p;

    mg_strlcpy(buf, path, sizeof(buf));
    fix_directory_separators(buf);

    /* Point p to the end of the file name */
    p = buf + strlen(buf) - 1;

    /* Trim trailing backslash character */
    while (p > buf && *p == '\\' && p[-1] != ':')
        *p-- = '\0';

    /*
     * Protect from CGI code disclosure.
     * This is very nasty hole. Windows happily opens files with
     * some garbage in the end of file name. So fopen("a.cgi    ", "r")
     * actually opens "a.cgi", and does not return an error!
     */
    if (*p == 0x20 || *p == 0x2e || *p == 0x2b || (*p & ~0x7f)) {
        (void) fprintf(stderr, "Rejecting suspicious path: [%s]", buf);
        buf[0] = '\0';
    }

    (void) MultiByteToWideChar(CP_UTF8, 0, buf, -1, wbuf, (int) wbuf_len);
    (void) WideCharToMultiByte(CP_UTF8, 0,wbuf, -1,  buf, FILENAME_MAX,NULL,NULL);
    remove_double_dots_and_double_slashes(buf);
    (void) MultiByteToWideChar(CP_UTF8, 0, buf, -1, wbuf, (int) wbuf_len); 
}

Original comment by larson....@gmail.com on 10 Aug 2010 at 3:22

GoogleCodeExporter commented 9 years ago
Issue 94 has been merged into this issue.

Original comment by valenok on 19 Sep 2010 at 9:13

GoogleCodeExporter commented 9 years ago
Hi all,

Well, I have tested the latest version (2.11) with the published 
vulnerabilities in the past, such as http://www.securityfocus.com/bid/34510, as 
well as the one discovered by Gera from CORE Security, and none worked.

Recently I and a friend published a specific fuzzer to find Directory Traversal 
vulnerabilities and I found a new flaw with this tool. This flaw affects 
Mongoose 2.11 only on Windows platforms.

The exploit code is shown below:
http://192.168.242.128:8080/%c0%2e%c0%2e/%c0%2e%c0%2e/%c0%2e%c0%2e/boot.ini

The fuzzer found the vulnerability only with that representation of dots 
(%c0%2e%c0%2).

Website of the fuzzer used: http://dotdotpwn.sectester.net

Regards.

Original comment by nitrouse...@gmail.com on 1 Nov 2010 at 2:13

Attachments:

GoogleCodeExporter commented 9 years ago
Hi, I'm the owner of http://code.google.com/p/onehttpd

Arrived here from Googling a similar issue that affected my project. It just 
occurred to me that the 3 line fix by larson would still be vulnerable to a 
"second level" attack when the dots become decoded again to "..". Not sure 
whether this is what nitrouse is reporting.

Original comment by sydneyf...@gmail.com on 14 Jun 2011 at 8:15

GoogleCodeExporter commented 9 years ago

Original comment by valenok on 21 Jun 2011 at 9:13

GoogleCodeExporter commented 9 years ago
Hi all,

Can somebody review this patch? I think it solves the conversion problems as it 
does a utf8->wide and reverse wide->utf8 and compares those. I could be wrong, 
but doesn't this fix the vulnerability?

Regards,

Maurits

Original comment by mevdsc...@gmail.com on 18 Jul 2011 at 11:37

Attachments:

GoogleCodeExporter commented 9 years ago
I tested this patch on Ubuntu 11.04 64bit cross-compiling using mingw32 and 
running it under wine-1.2.2. I also fixed the bug on line 914 that says "buf[0] 
= '\0';" and should say "wbuf[0] = '\0';" because otherwise it wouldn't fail. I 
also tested the attack string from nitrouse and the patch from larson. I found 
that the fix by larson seemed to work too, but my fix may be safer because it 
doesn't suffer from the (potential) problem sydney describes. @sydney: can you 
provide an example of such a "second level" attack? I would be happy to test 
and debug it.
-- Regards, Maurits

Original comment by mevdsc...@gmail.com on 19 Jul 2011 at 12:14

GoogleCodeExporter commented 9 years ago
I don't exactly remember what I meant by "second level" attack (my memory is 
really bad ~_~) I guess what I meant was if somebody could craft a path that is 
similar to the original exploit path when filtered through a UTF8->MB->UTF8 
conversion, then there's still a problem. Thinking more, I guess I might be 
wrong there, but I didn't have as much information when I wrote the comments.

Anyway, what I really wanted to add in this comment is that the MB conversion 
problem seems to *only* exist on Windows XP systems (not 2k, Win7 even wine, 
haven't tested on Vista though). You may want to get a real XP installation to 
test it.

There's also some more information about the multibytetowidechar problem in 
http://www.coresecurity.com/content/advisory-vmware

The C program used to exhaust the behavior of MultibyteToWideChar function 
seems quite interesting (though I needed to make minor modifications for it to 
work for me).

Original comment by sydneyf...@gmail.com on 19 Jul 2011 at 1:38

GoogleCodeExporter commented 9 years ago
Could you make a clone and put your changes in there, please?
It is easier to review and integrate that way.

Original comment by valenok on 19 Jul 2011 at 3:57

GoogleCodeExporter commented 9 years ago
Sure, and although I don't see what makes "patch mongoose.c mongoose.c.patch" 
less easy, I have applied the patch on clone "mevdschee-issue-90". I am looking 
forward to your review. Regards, Maurits

Original comment by mevdsc...@gmail.com on 19 Jul 2011 at 8:06

GoogleCodeExporter commented 9 years ago
I do not agree that issue 94 is a duplicate of this one, nor do I think issue 
94 is resolved by lines 905-915 in mongoose.c commented "// Protect from CGI 
code disclosure."

The main reason for issue 94 is that both "stat" and "fopen" on Windows allow a 
trailing backslash on a filename. This problem is still open!

Original comment by mevdsc...@gmail.com on 19 Jul 2011 at 11:12

GoogleCodeExporter commented 9 years ago
Having your code in the branch makes it easy to comment.
Also, your code does not follow a mongoose style. And you do not use unified 
diff, which makes reviewing your diff not easy.

Original comment by valenok on 20 Jul 2011 at 8:53

GoogleCodeExporter commented 9 years ago
is this issue fixed in latest version  ?
if not how can I produce that on my end ?

Original comment by goelvive...@gmail.com on 13 Aug 2011 at 12:38

GoogleCodeExporter commented 9 years ago
I have tested this issue on Windows XP pro SP3 and cant reproduce it there.

Original comment by mevdsc...@gmail.com on 18 Aug 2011 at 9:43

GoogleCodeExporter commented 9 years ago
Submitted 
https://code.google.com/p/mongoose/source/detail?r=b64155442daca61cabccefe2ec7e7
899a7d38d1b, thanks to
Please verify the fix, the development binary has been uploaded.

Original comment by valenok on 24 Aug 2011 at 11:57