Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#5402 closed defect (fixed)

#5090 allows post-auth heap overflow

Reported by: arma Owned by:
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: nextgens@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Robert Connolly from Matta Consulting reports that the config parsing bug in #5090 (which he found independently) is exploitable.

Examples for triggering it include

SETCONF aaaa="bbbbbbbbbbbb\x"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb

and setting a torrc line of

ContactInfo "Here I \x"amaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

Fortunately, it looks like it can only be triggered once you've authenticated to the control port (in which case you can already screw the user) or if you can edit the torrc file (same). So it's not harmful.

Is that really true? Are there any other instances of the bug? Any other ways of reaching it? Does the different trust model for the control socket change the above paragraph?

Jake opened #5210 to avoid future things like this being exploitable in practice.

And Nick points out:

We ought to audit uses of get_escaped_string_length in control.c
nevertheless.  It is a *PHENOMINALLY BAD IDEA* in C to have
independent functions that count the length of something and that
parse it.  We should look for other cases of that antipattern too.

(This ticket started out as a secteam discussion, until we decided that it didn't look like a remote execution bug.)

Child Tickets

Change History (7)

comment:1 Changed 8 years ago by arma

Robert requested that we put a CVE number on this. I'm waiting for him to send us one, then I'll put it in the changelog stanza.

comment:2 in reply to:  description Changed 8 years ago by rransom

Replying to arma:

And Nick points out:

We ought to audit uses of get_escaped_string_length in control.c
nevertheless.  It is a *PHENOMINALLY BAD IDEA* in C to have
independent functions that count the length of something and that
parse it.  We should look for other cases of that antipattern too.

get_escaped_string_length in control.c is harmless -- it is only called from the two functions which immediately follow it in the file (extract_escaped_string, which does not attempt to unescape the string, and decode_escaped_string, which unescapes the string using the same rules that get_escaped_string_length uses).

decode_escaped_string is only called from handle_control_authenticate, which can obviously be called before authenticating to the control port.

comment:3 in reply to:  1 Changed 8 years ago by nextgens

Cc: nextgens@… added

Replying to arma:

Robert requested that we put a CVE number on this. I'm waiting for him to send us one, then I'll put it in the changelog stanza.

CVE-2012-1668 has been assigned to this issue

comment:4 in reply to:  description Changed 8 years ago by nickm

Replying to arma:

Fortunately, it looks like it can only be triggered once you've authenticated to the control port (in which case you can already screw the user) or if you can edit the torrc file (same). So it's not harmful.

This line of reasoning is mostly true, but there are exceptions. For example, suppose that somebody has made a custom-built controller or torrc-generator program that accepts potentially hostile input but doesn't escape it correctly before passing it to Tor. I don't know of any such programs in use, but if there are, that would be one way to exploit this.

comment:5 Changed 8 years ago by arma

Resolution: fixed
Status: newclosed

Ok. I added the CVE, plus credit to "Robert Connolly from Matta Consulting", in git commit bca8bf62c66.

Closing as resolved. Please reopen if you think of any security complications we need to reconsider.

comment:6 Changed 7 years ago by nickm

Keywords: tor-client added

comment:7 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.