Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5909 closed defect (fixed)

wcstombs() doesn't guarantee NUL-termination (format_win32_error())

Reported by: asn Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

[censored] points out that wcstombs() does not guarantee NUL-termination of the output buffer.

He thinks that with a sufficiently large error message in format_win32_error(), and if UNICODE is set, tor_strdup() will also copy chunks of the stack.

Child Tickets

Change History (8)

comment:1 Changed 8 years ago by nickm

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

I'm pretty sure that we didn't build with -DUNICODE, and as such the only wcstombs should be the one in eventdns.c. Sebastian is too.

Forcing the output to be NUL-terminated should be simple, though I'm pretty surprised if we truly are getting an error message of > 512 wide characters.

Nevertheless, wrong code is wrong code. Added branch "bug5909_022" to resolve this by making buffers as big as needed, and by manually nul-terminating everything in case my buffer logic turns out to be wrong.

comment:2 Changed 8 years ago by nickm

Instead see bug5909_022_v2 ; lots of fixups there. I shouldn't write patches before coffee.

comment:3 Changed 8 years ago by nickm

Hm. I wonder if the right 0.2.2.x fix isn't just to disable building with -DUNICODE, and we should instead merge this into 0.2.3.x.

Still needs review.

comment:4 Changed 8 years ago by nickm

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

This is tricky and big enough that rebasing it into 0.2.3 seems smarter. Done as "bug5909_023". Still needs review.

comment:5 Changed 8 years ago by nickm

(Ha. We don't even build on windows with -DUNICODE -D_UNICODE; see bug #6097)

comment:6 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Still looks good to me. Merged it.

comment:7 Changed 8 years ago by nickm

Keywords: tor-client added

comment:8 Changed 8 years ago by nickm

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