#30561 closed defect (fixed)

Fixed tor_vasprintf on systems without vasprintf.

Reported by: paldium Owned by:
Priority: Low Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version: Tor:
Severity: Minor Keywords: 029-backport, 035-backport, 040-backport, dgoulet-merge, 034-unreached-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: nickm Sponsor:


If tor is compiled on a system with neither vasprintf nor _vscprintf,
the fallback implementation exposes a logic flaw which prevents
proper usage of strings longer than 127 characters:

  • tor_vsnprintf returns -1 if supplied buffer is not large enough, but tor_vasprintf uses this function to retrieve required length
  • the result of tor_vsnprintf is not properly checked for negative return values

Both aspects together could in theory lead to exposure of uninitialized
stack memory in the resulting string. This requires an invalid format
string or data that exceeds integer limitations.

Fortunately tor is not even able to run with this implementation because
it runs into asserts early on during startup. Also the unit tests fail
during a "make check" run.

At this point it would make sense to check if support for these systems is still a desired option. It seems that nobody noticed lack of support for at least a year.

Child Tickets

Attachments (1)

Fixed-tor_vasprintf-on-systems-without-vasprintf.patch (2.0 KB) - added by paldium 17 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 17 months ago by catalyst

Milestone: Tor: unspecified

comment:2 Changed 17 months ago by nickm

Status: newneeds_review

comment:3 Changed 17 months ago by nickm

Reviewer: nickm

comment:4 Changed 17 months ago by nickm

Keywords: 029-backport? 034-backport? 035-backport? 040-backport? added
Milestone: Tor: unspecifiedTor: 0.4.1.x-final

Paldium -- can you give an example of a system like this that I might be able to find and use for testing?

comment:5 Changed 17 months ago by nickm

Keywords: dgoulet-merge added
Status: needs_reviewmerge_ready

The patch looks okay to me. I've made some branches to fix the formatting, add a changes file, backport to 0.2.9, and add a note about removing this case from the code. I've confirmed that if I manually disable the first two implementations of tor_vasprintf(), the code now works and passes its test.

For 0.2.9 through 0.3.4: branch ticket30561_029 with PR at https://github.com/torproject/tor/pull/1052

For 0.3.5 and onward: branch ticket30561_035 with PR at https://github.com/torproject/tor/pull/1054

comment:6 Changed 17 months ago by dgoulet

Milestone: Tor: 0.4.1.x-finalTor: 0.4.0.x-final

The 035 branch (PR 1054) merged into master. Going to 040 milestone for backport.

comment:7 Changed 17 months ago by paldium

Hi nickm,

sorry for the late reply. I have encountered this code through manual review and I had no system around which lacks vasprintf. Therefore I have disabled the other two implementations manually as well.

The only system that is POSIX and lacks vasprintf is a closed-source Solaris version. Due to this lack of available test platforms I came up with the recommendation to drop this code completely.

Thanks for merging. :)

comment:8 Changed 17 months ago by nickm

Keywords: 029-backport? 034-backport? 035-backport? 040-backport? dgoulet-merge029-backport?, 034-backport?, 035-backport?, 040-backport?, dgoulet-merge

Removing 034-backport from all open tickets: 034 has reached EOL.

comment:9 Changed 15 months ago by teor

Keywords: 034-unreached-backport-maybe added; 034-backport? removed

0.3.4 is unsupported, we won't be backporting anything else to it.

comment:10 Changed 15 months ago by teor

Keywords: 029-backport 035-backport 040-backport 034-unreached-backport added; 029-backport? 035-backport? 040-backport? 034-unreached-backport-maybe removed
Milestone: Tor: 0.4.0.x-finalTor: 0.2.9.x-final
Resolution: fixed
Status: merge_readyclosed
Version: Tor:

Merged to 0.2.9 and 0.3.5.

Note: See TracTickets for help on using tickets.