about summary refs log tree commit diff
path: root/users/Profpatsch/netencode/netencode.rs
diff options
context:
space:
mode:
authorProfpatsch <mail@profpatsch.de>2022-01-29T11·50+0100
committerProfpatsch <mail@profpatsch.de>2022-02-14T14·12+0000
commited68ba675191bc3da57019b14609deb713247821 (patch)
tree222c6fadbfab3427688d89e01680061df83df2a2 /users/Profpatsch/netencode/netencode.rs
parent82ba42c4396c35e8bf69535af311e9b9f0cffa17 (diff)
feat(users/Profpatsch/netencode): ignore earlier record entries r/3822
It turns out that the netencode spec requiring to ignore *later*
entries meant that every parser has to do an extra check for each
element, instead of just overriding the key in the hash map.

This leads to a situation where the simple implementation is the wrong
one, which would lead to very subtle problems in parsers (see also the
infamous “json duplicate record entry” problem which has been used for
various exploits in the past).

To be fair, exploits are still possible, but at least a `Map.fromList`
will be the right implementation (provided it folds from the left) now
instead of the wrong one.

Examples of the trivial implementation being now right:

Python:

    > dict([("foo", 1), ("foo", 2)])
    {'foo': 2}

Rust:

    > println!("{:?}", HashMap::from([
      ("foo", 1),
      ("foo", 2)
    ]));
    {"foo": 2}

Haskell:

    > Data.Map.fromList [ ("foo", 1), ("foo", 2) ]
    fromList [("foo",2)]

Change-Id: Ife9593956f4718e5e720f4f348c227e4f3a71e2d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/5108
Tested-by: BuildkiteCI
Reviewed-by: Profpatsch <mail@profpatsch.de>
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: Profpatsch <mail@profpatsch.de>
Diffstat (limited to 'users/Profpatsch/netencode/netencode.rs')
-rw-r--r--users/Profpatsch/netencode/netencode.rs8
1 files changed, 3 insertions, 5 deletions
diff --git a/users/Profpatsch/netencode/netencode.rs b/users/Profpatsch/netencode/netencode.rs
index 0d92cf1ed4..bb08dca4aa 100644
--- a/users/Profpatsch/netencode/netencode.rs
+++ b/users/Profpatsch/netencode/netencode.rs
@@ -405,11 +405,9 @@ pub mod parse {
                     inner_no_empty_string(tag_g(&inner)),
                     HashMap::new(),
                     |mut acc: HashMap<_, _>, Tag { tag, mut val }| {
-                        // ignore duplicated tag names that appear later
+                        // ignore earlier tags with the same name
                         // according to netencode spec
-                        if !acc.contains_key(tag) {
-                            acc.insert(tag, *val);
-                        }
+                        let _ = acc.insert(tag, *val);
                         acc
                     },
                 ),
@@ -633,7 +631,7 @@ pub mod parse {
                 record_t("{25:<1:a|u,<1:b|u,<1:a|i1:-1,}".as_bytes()),
                 Ok((
                     "".as_bytes(),
-                    vec![("a".to_owned(), T::Unit), ("b".to_owned(), T::Unit),]
+                    vec![("a".to_owned(), T::I3(-1)), ("b".to_owned(), T::Unit),]
                         .into_iter()
                         .collect::<HashMap<_, _>>()
                 )),