Opened 9 months ago

Closed 5 months ago

#21507 closed defect (fixed)

Reject Tor versions that contain non-numeric prefixes

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 029-backport, 030-backport
Cc: Actual Points: 0.3
Parent ID: Points: 0.2
Reviewer: Sponsor:

Description

In #21450, we made tor_version_parse use:

tor_parse_uint64(... , 10, 0, INT32_MAX, ...);

But this still allows versions that start with:

an arbitrary amount of white space (as determined by isspace(3)) followed by a single optional "+" or "-" sign

This doesn't fit the version spec, and we should fix it.
This is an issue that's existed since we started parsing tor versions.

Child Tickets

Change History (8)

comment:1 Changed 9 months ago by teor

This is also important because of whitespace parsing differences in #21508.

comment:2 Changed 9 months ago by teor

Actual Points: 0.3
Keywords: 029-backport 030-backport added
Milestone: Tor: 0.3.0.x-finalTor: 0.2.9.x-final
Status: newneeds_review

Please see my github branch bug21507-029.

comment:3 Changed 9 months ago by teor

#21470 contains unit tests for this.

comment:4 Changed 9 months ago by dgoulet

Milestone: Tor: 0.2.9.x-finalTor: 0.3.1.x-final
Status: needs_reviewmerge_ready

Branch lgtm;

Actually now that master has become 031, I believe we should put this in 031 and then consider it for backport else it will get somehow forgotten in 029... I'm not entirely sure if this fix qualifies for a stable backport. If we think 029 is a valid backport then 025 should also be considered (LTS) but I'm not convinced of that.

comment:5 in reply to:  4 ; Changed 9 months ago by teor

Replying to dgoulet:

Branch lgtm;

Actually now that master has become 031, I believe we should put this in 031 and then consider it for backport else it will get somehow forgotten in 029... I'm not entirely sure if this fix qualifies for a stable backport. If we think 029 is a valid backport

0.2.9 is a valid backport so that authorities can filter badly formatted versions from the network. The reasons are the same as those for #21450 and the authority parts of #21278.

then 025 should also be considered (LTS) but I'm not convinced of that.

No authorities run any version earlier than 0.2.9.

comment:6 in reply to:  5 Changed 9 months ago by dgoulet

Replying to teor:

Replying to dgoulet:

Branch lgtm;

Actually now that master has become 031, I believe we should put this in 031 and then consider it for backport else it will get somehow forgotten in 029... I'm not entirely sure if this fix qualifies for a stable backport. If we think 029 is a valid backport

0.2.9 is a valid backport so that authorities can filter badly formatted versions from the network. The reasons are the same as those for #21450 and the authority parts of #21278.

Ok agree. We expect dir auth to be either on the latest stable or one before and in this case 029 (and very soon 030).

then 025 should also be considered (LTS) but I'm not convinced of that.

No authorities run any version earlier than 0.2.9.

Indeed.

comment:7 Changed 9 months ago by nickm

Milestone: Tor: 0.3.1.x-finalTor: 0.2.9.x-final

Merged to master. If no problems are encountered, we can backport to 0.2.9

comment:8 Changed 5 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Backported to 0.2.9 and merged forward to 0.3.0.

Note: See TracTickets for help on using tickets.