Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#6833 closed defect (fixed)

Tor authorities don't handle votes with more than 64 known-flags

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: 023-da tor-auth
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When we get more than 64 known-flags in a vote, we try to stuff them all in an uint64 anyway, and access them by masking with 1<<flagnum. That's undefined behavior right there.

For now, authorities should refuse to accept any vote with more than 64 flags. (We're not even close to that limit right now.) Later, we can use a bitfield instead or something.

This is only a vote issue. Consensus parsing doesn't have this problem, since we just look for the flags we recognize when we're parsing a consensus.

Child Tickets

Change History (9)

comment:1 Changed 8 years ago by nickm

Status: newneeds_review

Please review branch "bug6833".

comment:2 in reply to:  1 Changed 8 years ago by rransom

Replying to nickm:

Please review branch "bug6833".

It'll fix the bug, but there should probably be an explicit comment near the smartlist_len(ns->known_flags) > MAX_KNOWN_FLAGS_IN_VOTE check explaining that it is there to prevent undefined behaviour in routerstatus_parse_entry_from_string.

comment:3 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Added a comment about why the check is needed , and about why it's only needed for non-consensus networkstatuses. Then merged. Thanks!

comment:4 Changed 8 years ago by arma

Resolution: fixed
Status: closedreopened
<ftp> MAX_KNOWN_FLAGS_IN_VOTE must be 32, mine i486 can't handle more than 5 bits. so it's actually 31 in values or 32 for definition.
<ftp> I guess you i7 can't handle more than 5 bits too.

(Feel free to re-close once you've looked at it.)

(On first glance, I don't see how a uint64_t can only hold 5 bits. But I didn't look at the original bug either.)

comment:5 Changed 8 years ago by rransom

2012-09-15 14:55:43 <ftp> is it matter what integer i is for (1 << i)? actually it's int in result.

comment:6 Changed 8 years ago by rransom

Resolution: fixed
Status: reopenedclosed

OK, this is a different, bigger bug. Moved to #6853.

comment:7 Changed 8 years ago by nickm

Keywords: 023-da added

comment:8 Changed 8 years ago by nickm

Keywords: tor-auth added

comment:9 Changed 8 years ago by nickm

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