[tor-bugs] #2317 [Tor Client]: Missing sanity checks for cbtnummodes consensus parameter

[Tor Bug Tracker & Wiki]

#2317: Missing sanity checks for cbtnummodes consensus parameter
------------------------+---------------------------------------------------
 Reporter:  Sebastian   |       Owner:                    
     Type:  defect      |      Status:  needs_review      
 Priority:  major       |   Milestone:  Tor: 0.2.1.x-final
Component:  Tor Client  |     Version:                    
 Keywords:              |      Parent:                    
------------------------+---------------------------------------------------

Comment(by arma):

 Replying to [comment:12 nickm]:
 > Please also have a look at parameters as used in maint-0.2.1.  I only
 see one instance of networkstatus_get_param, but it should get audited.

 It looks solid. networkstatus_get_param() returns an int32_t, and checks
 via tor_parse_long() for a value between INT32_MIN and INT32_MAX. The
 result is written in circuit_initial_package_window() into an int32_t,
 which then does its own slightly tighter bounds checking.

 circuit_initial_package_window() then returns an int32_t, which is written
 into an int for the various package_window elements.

 So unless there's a platform where int can't fit an int32_t, I think we're
 in good shape.

 There is a way to cause an overflow though, which is to send an exit relay
 20 million sendme cells. At that point package_window will go negative,
 and the exit relay will assert. I'm not particularly worried, but at some
 point we might consider capping package_window at the value of
 circuit_initial_package_window().

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