Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#5090 closed defect (fixed)

parse_config_line_from_str accepts broken hex values

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

Description

parse_config_line_from_str happily accepts a broken hex value like "\xg4" and produces a broken value.

Expected behaviour: parse_config_line_from_str returns NULL as it does with broken octal values.

Child Tickets

Attachments (1)

util.c.diff (2.1 KB) - added by flupzor 8 years ago.
This should fix the mentioned bug.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by rransom

Component: - Select a componentTor Client
Priority: trivialminor

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

Status: newneeds_review

Replying to emanchado:

parse_config_line_from_str happily accepts a broken hex value like "\xg4" and produces a broken value.

Expected behaviour: parse_config_line_from_str returns NULL as it does with broken octal values.

Attached diff should fix this.

Changed 8 years ago by flupzor

Attachment: util.c.diff added

This should fix the mentioned bug.

comment:3 in reply to:  2 Changed 8 years ago by flupzor

Cc: alex@… added

Replying to flupzor:

Attached diff should fix this.

I forgot to mention that it also fixes a typo and makes the parser a bit stricter.

comment:4 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-final
Priority: minormajor

This looks good to me. I want to get emanchado's unit tests merged first, but this should go in 0.2.3.x.

comment:5 Changed 8 years ago by nickm

Actually, it's a bug; it should go in 0.2.2.x.

comment:6 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.2.x-final

comment:7 Changed 8 years ago by Sebastian

When merging, please fix the tab to be spaces. Otherwise, this seems fine imo

comment:8 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged into 0.2.2; thanks!

comment:9 Changed 8 years ago by arma

Note: in #5402 we point out potential security issues from this bug.

comment:10 Changed 7 years ago by nickm

Keywords: tor-client added

comment:11 Changed 7 years ago by nickm

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