Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#3306 closed defect (fixed)

crypto_rand_int() should be returning an unsigned int

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

Description

crypto_rand_int() is returning an int, when it should be returning an unsigned int.

Currently, if max is larger than INT_MAX it might return a negative number. Fortunately, it doesn't seem to be called with anything that large atm.

Child Tickets

Change History (10)

comment:1 Changed 9 years ago by asn

Status: newneeds_review

I made a patch for this in branch bug3306 in my mytor repo.
git://gitorious.org/mytor/mytor.git

comment:2 Changed 9 years ago by nickm

Milestone: Tor: 0.2.3.x-final

comment:3 Changed 9 years ago by rransom

wanoskarnet points out that in circuit_build_times_shuffle_and_store_array, crypto_rand_int can return a negative number, thereby possibly crashing Tor, if the CBT data in its state file is modified. We probably don't care about that causing Tor to exit, but it should assert rather than segfaulting.

comment:4 Changed 9 years ago by nickm

Roger notes that maybe it would make more sense to just check the input value rather than making sure that it's okay to replace every output with an unsigned int.

comment:5 Changed 9 years ago by nickm

Smaller fix in branch bug3306_nm in my public repository.

comment:6 Changed 9 years ago by arma

bug3306_nm looks plausible to me, assuming nickm continues liking it

comment:7 Changed 9 years ago by asn

yeah bug3306_nm is indeed more sensible.
commit b30b0dc0a871b781649558d8b66d93f90f13df2c seems correct to me.

comment:8 Changed 9 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.2.x-final
Resolution: fixed
Status: needs_reviewclosed

Great; merging (into 0.2.2, since it fixes a bug)

comment:9 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:10 Changed 7 years ago by nickm

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