Bug 631307 (CVE-2011-0063)

Possible to bypass fix for CVE-2011-0049 (majordomo2 directory traversal in 'help' command)

VERIFIED FIXED

Status

--
critical
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: reed, Assigned: justdave)

Tracking

({sec-critical, wsec-input})

other
sec-critical, wsec-input
Dependency tree / graph
Bug Flags:
sec-bounty +

Details

(Whiteboard: [infrasec:input][ws:critical])

Nikolas Sotiriu <nsotiriu@sotiriu.de> reported the following issue to security@ concerning a way to bypass the majordomo2 fix in bug 628064:

=================================================================
So the bug is that the majordomo2 path for the bug (628064) is absolut
terrible and not working.

See attachment 506481 [details] [diff] [review].

Check the regex ($file =~ s!/?\.\./?!!g;) do you see it :)

It deletes ../ but what happens if i ./.../ ?

./.../ becomes ../

http://bugzilla.org/cgi-bin/mj_wwwusr?passw=&list=GLOBAL&user=&func=help&extra=./..././..././..././..././..././..././..././.../etc/passwd

Maybe a regex like this is better:

$file =~ s/\.\.//g;
=================================================================

Comment 1

8 years ago
"absolut terrible and not working".  Wow.  I'm in here helping folks fix code I haven't worked on in years, and honestly think I deserve something better than abuse.

Now, great, that's clever and I never would have found it.  Sure wish someone could just communicate with me directly instead of flaming me in bugs I'm lucky to actually see.  I've committed a fix.
He mailed it to our security@, probably didn't know you were on the bug (didn't post it there anyway).  Doesn't excuse the "verbal abuse" anyway.  I still appreciate you taking the time to help with this. :)
Upstream patch pulled and applied.

For the record:

Index: lib/Majordomo.pm
===================================================================
RCS file: /home/cvs/majordomo/majordomo/lib/Majordomo.pm,v
retrieving revision 1.445
retrieving revision 1.446
diff -u -r1.445 -r1.446
--- lib/Majordomo.pm    24 Jan 2011 20:33:54 -0000      1.445
+++ lib/Majordomo.pm    3 Feb 2011 19:42:07 -0000       1.446
@@ -3521,7 +3521,7 @@
   my $lang = $args{lang};
 
   # $file might have bad stuff in it
-  $file =~ s!/?\.\./?!!g;
+  $file =~ s!\.\.!!g;
 
   # Account for list:sublist
   if ($list =~ /^([^:\s]+):/) {
er, sorry, that diff is backwards.  swap the + and - as you read it :)
actually, on further poking, I think that's actually the correct order.
Yeah, the currently live code has:
  $file =~ s!\.\.!!g;
Duplicate of this bug: 631309
I'm still not 100% sure on that regexp.  I know I've seen this done in other apps and I'm pretty sure the regexp was different.  But I can't put my finger on it, and I haven't managed to break this one yet.

Comment 10

8 years ago
Well, there are other possibilities.  Getting too clever by half in trying to strip out dots next to slashes obviously wasn't the right thing to do.  If you come up with something better I'm happy to use it.

http://stackoverflow.com/questions/1734145/is-this-specific-path-concatenation-in-perl-code-exploitable seems to be somewhat on point.  I could also detect any double-dot anywhere and instead of trying to manipulate the string just replace thw whole thing with the error document.

Anyway, always open to suggestions.  I make no claims at all to being any kind of security professional.
No problem, thanks for all the help so far, you've been super responsive for someone that's been mostly away from the project for a few years. :)

Guess I'll own this for now, it's paging for being unassigned for too long at critical severity. :)
Assignee: server-ops → justdave

Comment 12

8 years ago
(In reply to comment #1)
> "absolut terrible and not working".  Wow.  I'm in here helping folks fix code I
> haven't worked on in years, and honestly think I deserve something better than
> abuse.
> 
> Now, great, that's clever and I never would have found it.  Sure wish someone
> could just communicate with me directly instead of flaming me in bugs I'm lucky
> to actually see.  I've committed a fix.


Sorry for that. i didn't know that this would make you so angry. It was not my intention to criticize your help to fix the problem.

The point was, that i'm not a perl programmer (just used it for some
exploits) but i directy saw that this patch didn't work.

So sorry again for the "verbal abuse"

BTW: Nice that the email was posted here, but the last passage is missing

> But i can't give any garanty that this will realy work, because i 
> don't have a majordomo installation here.

It looks like a new snapshot is out, which has the fixed regex http://ftp.mj2.org/pub/mj2/snapshots/2011-02/majordomo-20110204.tar.gz

What about the public disclosure?

Advisory draft: http://sotiriu.de/adv/NSOADV-2011-1268349720915729430439.txt
Alias: CVE-2011-0063

Comment 14

8 years ago
Can this bug be made public, to inform the majordomo users to patch there systems?

Comment 15

8 years ago
justdave and cshields,  can you get someone to mark this fixed, do the verfication including possible side cases.

then it seem appropriate for us to start the disclosure process so other users can start patching.
Fix has been live on our site for a while, we were just waiting to determine if it was still exploitable by any means, and we haven't found anything yet.  opsec should verify, they're better at the xss testcases.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
FWIW, this has already been disclosed to most majordomo2 users via the mj2-dev mailing list (which since it never actually got released, the majority of the admins are subscribed to it as the only real source of info about it) on February 5th.    http://comments.gmane.org/gmane.mail.majordomo.majordomo2.devel/4057

Comment 18

8 years ago
(In reply to comment #16)
> opsec should verify, they're better at the xss testcases.

Should i wait for this verification? or is it ok to release the advisory?
I can't find obvious methods of bypassing the regex, but that doesn't provide a lot of assurance since there are so many edge cases.

Let me turn this approach around a bit. Could we specify a whitelist of allowed names for the "extra" parameter? For example [a-zA-Z0-9]?  We could even through in the / character to support subdirectories.

This approach provides a lot more confidence.

Comment 20

8 years ago
my $id = $cgi->param('id');
if ($id) {
    # Be careful not to allow directory traversal.
    if ($id =~ /\.\./) {
        # two dots in a row is bad
        ThrowCodeError("bad_page_cgi_id", { "page_id" => $id });
    }
    # Split into name and ctype.
    $id =~ /^([\w\-\/\.]+)\.(\w+)$/;
    if (!$2) {
        # if this regexp fails to match completely, something bad came in
        ThrowCodeError("bad_page_cgi_id", { "page_id" => $id });
    }

This is how bugzilla is handling the directory traversal problem.
(In reply to comment #20)
I'd argue against a more complex regex. This becomes more difficult to reliably verify. Also, this code appears to be serving a slightly different purpose.

Comment 22

8 years ago
I just posted it, that you can see, that bugzilla use the currently used regex. And it's working.

Comment 23

8 years ago
The Status is RESOLVED FIXED.

Can i release the advisory now?
(In reply to comment #23)
> The Status is RESOLVED FIXED.
> 
> Can i release the advisory now?

We need to mark this as "Verified" before we can publicly release any info.  I'll have this wrapped up by tomorrow (3/8/11)
The solution is not ideal, but we can't identify any vectors to bypass the solution.

Issue verified. Fix in prod and confirmed.  This can be released for advisories.
Status: RESOLVED → VERIFIED
(In reply to comment #15)
> justdave and cshields,  can you get someone to mark this fixed, do the
> verfication including possible side cases.
> 
> then it seem appropriate for us to start the disclosure process so other users
> can start patching.

Chris,  Who is coordinating the disclosure?

Comment 27

8 years ago
is that a question for me chris, or clyon chris, or other chris? ;-)

seems like someone from majordomo would be the best candidate to coordinate and do the disclosure.
Sorry, question was for you (Chofmann) per your comment 15

Comment 29

8 years ago
(In reply to comment #25)
> The solution is not ideal, but we can't identify any vectors to bypass the
> solution.
> 
> Issue verified. Fix in prod and confirmed.  This can be released for
> advisories.

Ok ... i will send the advisory today to full-disc...

Updated

8 years ago
Group: websites-security
(In reply to comment #27)
> seems like someone from majordomo would be the best candidate to coordinate and
> do the disclosure.

As mentioned in comment 17, it was already disclosed via Majordomo2's channels on February 5th.

Updated

7 years ago
Component: Server Operations: Security → Security Assurance: Operations

Updated

6 years ago
Blocks: 836522
Flags: sec-bounty+
Component: Operations Security (OpSec): General → General
Product: mozilla.org → Enterprise Information Security
Keywords: sec-critical, wsec-input
You need to log in before you can comment on or make changes to this bug.