commit 3efe53562fea2f4d71f0f620e10f4b5319b99fcb Author: teor teor@riseup.net Date: Mon May 18 21:50:35 2020 +1000

rust/protover: Fix protocol version support error handling

Make Rust protocol version support checks consistent with the undocumented error behaviour of the corresponding C code.

Fixes bug 34251; bugfix on 0.3.3.5-rc. --- changes/bug34251 | 4 ++++ src/core/or/protover.c | 9 +++++++++ src/rust/protover/ffi.rs | 10 +++++----- 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/changes/bug34251 b/changes/bug34251 new file mode 100644 index 000000000..bbf053525 --- /dev/null +++ b/changes/bug34251 @@ -0,0 +1,4 @@ + o Minor bugfixes (rust, protocol versions): + - Make Rust protocol version support checks consistent with the + undocumented error behaviour of the corresponding C code. + Fixes bug 34251; bugfix on 0.3.3.5-rc. diff --git a/src/core/or/protover.c b/src/core/or/protover.c index 43fb67772..c6b024369 100644 --- a/src/core/or/protover.c +++ b/src/core/or/protover.c @@ -326,6 +326,9 @@ protover_is_supported_here(protocol_type_t pr, uint32_t ver) /** * Return true iff "list" encodes a protocol list that includes support for * the indicated protocol and version. + * + * If the protocol list is unparseable, treat it as if it defines no + * protocols, and return 0. */ int protocol_list_supports_protocol(const char *list, protocol_type_t tp, @@ -348,6 +351,9 @@ protocol_list_supports_protocol(const char *list, protocol_type_t tp, /** * Return true iff "list" encodes a protocol list that includes support for * the indicated protocol and version, or some later version. + * + * If the protocol list is unparseable, treat it as if it defines no + * protocols, and return 0. */ int protocol_list_supports_protocol_or_later(const char *list, @@ -740,6 +746,9 @@ protover_compute_vote(const smartlist_t *list_of_proto_strings, * one that we support, and false otherwise. If <b>missing_out</b> is * provided, set it to the list of protocols we do not support. * + * If the protocol version string is unparseable, treat it as if it defines no + * protocols, and return 1. + * * NOTE: This is quadratic, but we don't do it much: only a few times per * consensus. Checking signatures should be way more expensive than this * ever would be. diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs index 14170d035..2bf8d3a98 100644 --- a/src/rust/protover/ffi.rs +++ b/src/rust/protover/ffi.rs @@ -84,7 +84,7 @@ pub extern "C" fn protocol_list_supports_protocol( version: uint32_t, ) -> c_int { if c_protocol_list.is_null() { - return 1; + return 0; }

// Require an unsafe block to read the version from a C string. The pointer @@ -93,7 +93,7 @@ pub extern "C" fn protocol_list_supports_protocol(

let protocol_list = match c_str.to_str() { Ok(n) => n, - Err(_) => return 1, + Err(_) => return 0, }; let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { Ok(n) => n, @@ -140,7 +140,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later( version: uint32_t, ) -> c_int { if c_protocol_list.is_null() { - return 1; + return 0; }

// Require an unsafe block to read the version from a C string. The pointer @@ -149,7 +149,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(

let protocol_list = match c_str.to_str() { Ok(n) => n, - Err(_) => return 1, + Err(_) => return 0, };

let protocol = match translate_to_rust(c_protocol) { @@ -159,7 +159,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(

let proto_entry: UnvalidatedProtoEntry = match protocol_list.parse() { Ok(n) => n, - Err(_) => return 1, + Err(_) => return 0, };

if proto_entry.supports_protocol_or_later(&protocol.into(), &version) {