]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: ebtree: Invalid read when looking for dup entry
authorRemi Tricot-Le Breton <rlebreton@haproxy.com>
Tue, 18 May 2021 16:56:42 +0000 (18:56 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 18 May 2021 17:26:21 +0000 (19:26 +0200)
The first item inserted into an ebtree will be inserted directly below
the root, which is a simple struct eb_root which only holds two branch
pointers (left and right).
If we try to find a duplicated entry to this first leaf through a
ebmb_next_dup, our leaf_p pointer will point to the eb_root instead of a
complete eb_node so we cannot look for the bit part of our leaf_p since
it would try to cast our eb_root into an eb_node and perform an out of
bounds access when reading "eb_root_to_node(eb_untag(t,EB_LEFT)))->bit".
This bug was found by address sanitizer running on a CRL hot update VTC
test.

Note that the bug has been there since the import of the eb_next_dup()
and eb_prev_dup() function in 1.5-dev19 by commit 2b5702030 ("MINOR:
ebtree: add new eb_next_dup/eb_prev_dup() functions to visit duplicates").

It can be backported to all stable branches.

include/import/ebtree.h

index 85ea31d9bf759e303cacc5a7bcd1edf3d6a28253..36fcef50773d4d083d159b418bab73bdca55d105 100644 (file)
@@ -615,12 +615,16 @@ static inline struct eb_node *eb_next_dup(struct eb_node *node)
                t = (eb_root_to_node(eb_untag(t, EB_RGHT)))->node_p;
        }
 
-       /* Note that <t> cannot be NULL at this stage */
+       /* Note that <t> cannot be NULL at this stage. If our leaf is directly
+        * under the root, we must not try to cast the leaf_p into a eb_node*
+        * since it is a pointer to an eb_root.
+        */
+       if (eb_clrtag((eb_untag(t, EB_LEFT))->b[EB_RGHT]) == NULL)
+               return NULL;
+
        if ((eb_root_to_node(eb_untag(t, EB_LEFT)))->bit >= 0)
                return NULL;
        t = (eb_untag(t, EB_LEFT))->b[EB_RGHT];
-       if (eb_clrtag(t) == NULL)
-               return NULL;
        return eb_walk_down(t, EB_LEFT);
 }