Undefined behavior in tor_vasprintf()

[asn]

Overflowing a signed integer in C is an undefined behaviour.
It is possible to trigger this undefined behaviour in tor_asprintf on
Windows or systems lacking vasprintf.

On these systems, eiter _vscprintf or vsnprintf is called to retrieve
the required amount of bytes to hold the string. These functions can
return INT_MAX. The easiest way to recreate this is the use of a
specially crafted configuration file, e.g. containing the line:

FirewallPorts AAAAA<in total 2147483610 As>

This line triggers the needed tor_asprintf call which eventually
leads to an INT_MAX return value from _vscprintf or vsnprintf.

The needed byte for \0 is added to the result, triggering the
overflow and therefore the undefined behaviour.

Casting the value to size_t before addition fixes the behaviour.

[asn]

Bug found by Tobias Stoeckmann.

Patch by Tobias can be found here: ​https://github.com/torproject/tor/pull/1144

[teor]

This patch makes sense to me, and it passes CI.

I'm marking it as security-low, because most common compilers don't aggressively optimise signed overflow in this context.
(If they did, this code could introduce some nasty bugs in tor.)

So the negative value will be converted to size_t by adding SIZE_T_MAX.
On 32-bit systems, that's the correct value, on 64-bit systems, that's UINT64_MAX - INT32_MIN, which will fail to malloc and crash.
Fortunately, most of Tor's parsers have document size limits that are much lower than 2GB.

But we still need to backport this fix to compact.c in 0.2.9, and then merge forward.

[nickm]

I'll do the backport

[nickm]

I have two branches here: bug31001_029 for maint-0.2.9, and bug31001_035 for maint-0.3.5 and later.

PR for 0.2.9: ​https://github.com/torproject/tor/pull/1178
PR for 0.3.5: ​https://github.com/torproject/tor/pull/1179
PR for master (for CI purposes): ​https://github.com/torproject/tor/pull/1180

[catalyst]

Replying to nickm:

I have two branches here: bug31001_029 for maint-0.2.9, and bug31001_035 for maint-0.3.5 and later.

PR for 0.2.9: ​https://github.com/torproject/tor/pull/1178
PR for 0.3.5: ​https://github.com/torproject/tor/pull/1179
PR for master (for CI purposes): ​https://github.com/torproject/tor/pull/1180

Thanks! These look good to me.

[nickm]

Please merge to 0.4.0 and forward, and mark for backport?

[dgoulet]

Merged in 041 and forward! Moving to 040 for backport.

[teor]

This PR was merged after 0.4.1.4, so we will test it until 0.4.1.6 is released, then merge.