[tor-bugs] #4233 [Tor Client]: Exact addition/summation and memory allocation

[Tor Bug Tracker & Wiki]

#4233: Exact addition/summation and memory allocation
-------------------------+--------------------------------------------------
 Reporter:  mansour      |          Owner:     
     Type:  enhancement  |         Status:  new
 Priority:  normal       |      Milestone:     
Component:  Tor Client   |        Version:     
 Keywords:               |         Parent:     
   Points:               |   Actualpoints:     
-------------------------+--------------------------------------------------
 I've been looking at the memory allocation functions in common/util.c and
 how they're used.

 Sometimes tor_malloc is used like so:

   fred = tor_malloc(foo + bar + baz);

 That sum could overflow, sometimes to zero -- in which case tor_malloc
 will return having allocated a single byte. That's to avoid problematic
 malloc(0) but this behaviour could also be problematic in case of overflow
 (e.g. if foo bytes are then memcpy-ed). This doesn't happen in practice
 probably because Tor is well written, but one never knows...

 So one solution is to do exact addition/summation before calling
 tor_malloc. That's the approach I took; here's an example instead of a
 novel:

   size_t size;
   tor_assert(tor_sum_sizet(&size, 3, foo, bar, baz) == 3);
   fred = tor_malloc(size);

 The tor_sum_sizet function stores the sum of foo, bar and baz into the
 size variable only if there's no overflow. Otherwise size is set to zero
 and the function returns the number of summands that would have been
 successfully summed, which should be the same as the second argument
 passed.

 Another thing is cases like:

   fred = tor_malloc(foo + 1);

 If there is overflow at all it'll be to zero -- so I propose that
 tor_malloc(0) fails as it would if it were to return NULL. There's no
 analogue to this in the C standard, but I don't see anywhere in Tor where
 tor_malloc(0) is used anyway. Also consider:

   size_t size;
   (void) tor_sum_sizet(&size, 2, foo, bar);
   fred = tor_malloc(size);

 Here if tor_sum_sizet fails, the size variable is set to 0, and tor_malloc
 then fails.

 I've implemented all the above in common/util.c. Please let me know if
 this is interesting (perhaps for a distant alpha version) and review if
 you have time. I'll try to write unit tests for it all.

 The last patch (3/3) is an illustration of all the above. I see about 50
 places in the code where the same approach could be useful.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/4233>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online