TROVE-2021-002: strstr() in dirvote_add_signatures_to_pending_consensus() should be find_str_at_start_of_line()
Found while doing #40229 (closed).
I think this means that if you nickname your relay "directory-signature", you can prevent any consensus from being generated and crash the authorities. Yowch!
- Show closed items
Activity
-
Newest first Oldest first
-
Show all activity Show comments only Show history only
- Nick Mathewson assigned to @nickm
assigned to @nickm
- Owner
Sounds like something where the authorities should get a heads up.
- Author Owner
Actually I think this attack won't work. @arma spotted:
#define LEGAL_NICKNAME_CHARACTERS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
I was looking at the wrong character list.
But is there any other way for a non-authority to get that into a consensus?
- Author Owner
Specifically it would have to be the string "directory-signature " with a trailing space.
- Author Owner
oh, yeah. An authority could do that trivially by setting it as their contact address.
- Author Owner
I think that the closest a relay can do is to claim to support the "directory-signature=1" protocol. Fortunately, that won't have the trailing space.
- Author Owner
Patch at https://gitlab.torproject.org/nickm/tor-private/-/merge_requests/3 . Let's keep this private until we confirm it is low-severity.
- Reporter
v line?
- Author Owner
oh, that would be bad.
- Author Owner
Yeah, I think that could be exploitable. The code for that is in version_from_platform(), which can be made to include an internal space.
- Author Owner
https://gitlab.torproject.org/nickm/tor-private/-/merge_requests/4 is the updated MR: It is now against an earlier branch and it describes the bug correctly.
Let's get authorities running this fix :)
- Nick Mathewson added Needs Review label
added Needs Review label
- Author Owner
This is now also CVE-2021-28090
- Nick Mathewson closed with commit 890ae4fb
closed with commit 890ae4fb
- Author Owner
I'll make this public and close it once the release has been out a few days; it has instructions for exploiting the issue.
- Nick Mathewson reopened
reopened
- Nick Mathewson added Merge Ready label and removed Needs Review label
added Merge Ready label and removed Needs Review label
- Nick Mathewson made the issue visible to everyone
made the issue visible to everyone
- Author Owner
Marking as unconfidential; the fix has been out since Tuesday.
- Nick Mathewson closed
closed