Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#2331 closed defect (fixed)

Possible integer overflows in base32_encode, base32_decode

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


doors reports that the loop-termination comparisons in base32_encode and base32_decode compare indices of type unsigned int with bounds of type size_t. The loops will never terminate if the upper bounds are greater than UINT_MAX.

I see two other, more direct integer overflows in those functions:

In base32_encode:

size_t nbits = srclen * 8;

In base32_decode:

size_t nbits;
nbits = srclen * 5;

In both functions, srclen is a parameter of type size_t.

Child Tickets

Change History (7)

comment:1 Changed 10 years ago by nickm

Milestone: Tor: 0.2.1.x-finalTor: 0.2.2.x-final
Priority: criticalnormal

Hm. Fortunately, we never use base32_encode/base32_decode for anything other than:

  • generating random hostnames from values of lengths that never approach UINT_MAX/8
  • manipulating hidden service IDs and secrets, where every use case involves a constant input length and a constant output length, neither of which approaches UINT_MAX/8.

So this isn't triggerable for now afaict. We should fix it anyway, of course.

comment:2 Changed 10 years ago by nickm

Status: newneeds_review

See branch bug2331 in my public.

comment:3 Changed 10 years ago by cypherpunks

fix a signed-unsigned
comparison there too.



unsigned int i, bit;
for (i = 0, bit = 0; bit < nbits; ++i, bit += 8) {

sizeof(bit) vs. sizeof(nbits), still there.

comment:4 Changed 10 years ago by nickm

Fixed; thanks.

comment:5 Changed 10 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

merged to 0.2.2 and master.

comment:6 Changed 8 years ago by nickm

Keywords: tor-relay added

comment:7 Changed 8 years ago by nickm

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