summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/bug271943
-rw-r--r--src/core/or/protover.c3
-rw-r--r--src/feature/dirparse/ns_parse.c41
-rw-r--r--src/rust/protover/protover.rs17
-rw-r--r--src/rust/protover/tests/protover.rs13
-rw-r--r--src/test/test_protover.c4
6 files changed, 53 insertions, 28 deletions
diff --git a/changes/bug27194 b/changes/bug27194
new file mode 100644
index 0000000000..a1919c6c49
--- /dev/null
+++ b/changes/bug27194
@@ -0,0 +1,3 @@
+ o Minor bugfixes (protover):
+ - Consistently reject extra commas, instead of only rejecting leading commas.
+ Fixes bug 27194; bugfix on 0.2.9.4-alpha.
diff --git a/src/core/or/protover.c b/src/core/or/protover.c
index 1ac264925c..22128dff18 100644
--- a/src/core/or/protover.c
+++ b/src/core/or/protover.c
@@ -250,7 +250,8 @@ parse_single_entry(const char *s, const char *end_of_entry)
}
s = comma;
- while (*s == ',' && s < end_of_entry)
+ // Skip the comma separator between ranges. Don't ignore a trailing comma.
+ if (s < (end_of_entry - 1))
++s;
}
diff --git a/src/feature/dirparse/ns_parse.c b/src/feature/dirparse/ns_parse.c
index d4bcdae973..726da8a64c 100644
--- a/src/feature/dirparse/ns_parse.c
+++ b/src/feature/dirparse/ns_parse.c
@@ -13,6 +13,7 @@
#include "core/or/or.h"
#include "app/config/config.h"
+#include "core/or/protover.h"
#include "core/or/versions.h"
#include "feature/client/entrynodes.h"
#include "feature/dirauth/dirvote.h"
@@ -466,6 +467,10 @@ routerstatus_parse_entry_from_string(memarea_t *area,
}
}
+ // If the protover line is malformed, reject this routerstatus.
+ if (protocols && protover_contains_long_protocol_names(protocols)) {
+ goto err;
+ }
summarize_protover_flags(&rs->pv, protocols, version);
}
@@ -1063,6 +1068,19 @@ extract_shared_random_srvs(networkstatus_t *ns, smartlist_t *tokens)
}
}
+/** Allocate a copy of a protover line, if present. If present but malformed,
+ * set *error to true. */
+static char *
+dup_protocols_string(smartlist_t *tokens, bool *error, directory_keyword kw)
+{
+ directory_token_t *tok = find_opt_by_keyword(tokens, kw);
+ if (!tok)
+ return NULL;
+ if (protover_contains_long_protocol_names(tok->args[0]))
+ *error = true;
+ return tor_strdup(tok->args[0]);
+}
+
/** Parse a v3 networkstatus vote, opinion, or consensus (depending on
* ns_type), from <b>s</b>, and return the result. Return NULL on failure. */
networkstatus_t *
@@ -1184,14 +1202,18 @@ networkstatus_parse_vote_from_string(const char *s,
}
}
- if ((tok = find_opt_by_keyword(tokens, K_RECOMMENDED_CLIENT_PROTOCOLS)))
- ns->recommended_client_protocols = tor_strdup(tok->args[0]);
- if ((tok = find_opt_by_keyword(tokens, K_RECOMMENDED_RELAY_PROTOCOLS)))
- ns->recommended_relay_protocols = tor_strdup(tok->args[0]);
- if ((tok = find_opt_by_keyword(tokens, K_REQUIRED_CLIENT_PROTOCOLS)))
- ns->required_client_protocols = tor_strdup(tok->args[0]);
- if ((tok = find_opt_by_keyword(tokens, K_REQUIRED_RELAY_PROTOCOLS)))
- ns->required_relay_protocols = tor_strdup(tok->args[0]);
+ // Reject the vote if any of the protocols lines are malformed.
+ bool unparseable = false;
+ ns->recommended_client_protocols = dup_protocols_string(tokens, &unparseable,
+ K_RECOMMENDED_CLIENT_PROTOCOLS);
+ ns->recommended_relay_protocols = dup_protocols_string(tokens, &unparseable,
+ K_RECOMMENDED_RELAY_PROTOCOLS);
+ ns->required_client_protocols = dup_protocols_string(tokens, &unparseable,
+ K_REQUIRED_CLIENT_PROTOCOLS);
+ ns->required_relay_protocols = dup_protocols_string(tokens, &unparseable,
+ K_REQUIRED_RELAY_PROTOCOLS);
+ if (unparseable)
+ goto err;
tok = find_by_keyword(tokens, K_VALID_AFTER);
if (parse_iso_time(tok->args[0], &ns->valid_after))
@@ -1453,6 +1475,7 @@ networkstatus_parse_vote_from_string(const char *s,
smartlist_add(ns->routerstatus_list, rs);
} else {
vote_routerstatus_free(rs);
+ goto err; // Malformed routerstatus, reject this vote.
}
} else {
routerstatus_t *rs;
@@ -1463,6 +1486,8 @@ networkstatus_parse_vote_from_string(const char *s,
flav))) {
/* Use exponential-backoff scheduling when downloading microdescs */
smartlist_add(ns->routerstatus_list, rs);
+ } else {
+ goto err; // Malformed routerstatus, reject this vote.
}
}
}
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index 076cd5301e..550732734c 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -253,6 +253,11 @@ impl FromStr for ProtoEntry {
/// Otherwise, the `Err` value of this `Result` is a `ProtoverError`.
fn from_str(protocol_entry: &str) -> Result<ProtoEntry, ProtoverError> {
let mut proto_entry: ProtoEntry = ProtoEntry::default();
+
+ if protocol_entry.is_empty() {
+ return Ok(proto_entry);
+ }
+
let entries = protocol_entry.split(' ');
for entry in entries {
@@ -501,6 +506,10 @@ impl UnvalidatedProtoEntry {
) -> Result<Vec<(&'a str, &'a str)>, ProtoverError> {
let mut protovers: Vec<(&str, &str)> = Vec::new();
+ if protocol_string.is_empty() {
+ return Ok(protovers);
+ }
+
for subproto in protocol_string.split(' ') {
let mut parts = subproto.splitn(2, '=');
@@ -859,7 +868,8 @@ mod test {
#[test]
fn test_protoentry_from_str_empty() {
- assert_protoentry_is_unparseable!("");
+ assert_protoentry_is_parseable!("");
+ assert!(UnvalidatedProtoEntry::from_str("").is_ok());
}
#[test]
@@ -883,11 +893,6 @@ mod test {
}
#[test]
- fn test_protoentry_from_str_() {
- assert_protoentry_is_unparseable!("");
- }
-
- #[test]
fn test_protoentry_all_supported_single_protocol_single_version() {
let protocol: UnvalidatedProtoEntry = "Cons=1".parse().unwrap();
let unsupported: Option<UnvalidatedProtoEntry> = protocol.all_supported();
diff --git a/src/rust/protover/tests/protover.rs b/src/rust/protover/tests/protover.rs
index 942fe3c6ab..c97810a6f2 100644
--- a/src/rust/protover/tests/protover.rs
+++ b/src/rust/protover/tests/protover.rs
@@ -70,18 +70,6 @@ fn protocol_all_supported_with_one_value() {
}
#[test]
-#[should_panic]
-fn parse_protocol_unvalidated_with_empty() {
- let _: UnvalidatedProtoEntry = "".parse().unwrap();
-}
-
-#[test]
-#[should_panic]
-fn parse_protocol_validated_with_empty() {
- let _: UnvalidatedProtoEntry = "".parse().unwrap();
-}
-
-#[test]
fn protocol_all_supported_with_three_values() {
let protocols: UnvalidatedProtoEntry = "LinkAuth=1 Microdesc=1-2 Relay=2".parse().unwrap();
let unsupported: Option<UnvalidatedProtoEntry> = protocols.all_supported();
@@ -156,7 +144,6 @@ fn parse_protocol_with_unexpected_characters() {
}
#[test]
-#[should_panic]
fn protover_compute_vote_returns_empty_for_empty_string() {
let protocols: &[UnvalidatedProtoEntry] = &["".parse().unwrap()];
let listed = ProtoverVote::compute(protocols, &1);
diff --git a/src/test/test_protover.c b/src/test/test_protover.c
index 5e74265550..4ccec73699 100644
--- a/src/test/test_protover.c
+++ b/src/test/test_protover.c
@@ -612,6 +612,10 @@ test_protover_vote_roundtrip(void *args)
{ "N-1=1,2", "N-1=1-2" },
{ "-1=4294967295", NULL },
{ "-1=3", "-1=3" },
+ { "Foo=,", NULL },
+ { "Foo=,1", NULL },
+ { "Foo=1,,3", NULL },
+ { "Foo=1,3,", NULL },
/* junk. */
{ "!!3@*", NULL },
/* Missing equals sign */
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion