]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: stick-tables: use the same bucket for string keys with a NUL
authorOlivier Houchard <ohouchard@haproxy.com>
Thu, 6 Aug 2026 07:30:13 +0000 (09:30 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 7 Aug 2026 08:29:19 +0000 (10:29 +0200)
String stick-table keys are stored NUL-terminated and looked up in a string
ebtree, so an entry is identified by the bytes preceding the first NUL.
Accordingly stksess_kill(), __stksess_kill_if_expired(), stktable_lookup(),
stktable_requeue_exp() and stktable_set_entry() all derive the bucket from
strlen() of the stored key, but stktable_lookup_key() and
stktable_get_entry() derive it from the raw sample length. A key carrying an
embedded NUL is thus inserted in one bucket and later killed or requeued
while holding the lock of another one, so that bucket's tree ends up
modified without its lock while other threads look it up: corrupted tree,
hence a crash, a lost or duplicated entry, or a use-after-free on a stksess.

Embedded NULs are not exotic: url_decode() turns "%00" into one and keeps
going, while smp_to_stkey() passes the sample length as-is. Tracking
"url_param(q),url_dec" into a string table and sending "GET /?q=AB%00CD"
yields key_len 5 but strlen 2, hence two different buckets. The peers
protocol also transports raw key bytes.

Let's make the two remaining places stop at the first NUL as well, so that
a single canonical length is used everywhere.

The bucket split was introduced in 3.0 by commit 1a088da7c ("MAJOR:
stktable: split the keys across multiple shards to reduce contention").
This must be backported to 3.0.

Reported-by: Claude (ANT-2026-TNFHK5ZG)
src/stick_table.c

index 6bc8f172f7be1397e898ab785882775b71afab8a..bf5e4fc6938ddff2b5bd3846bce15731809a212f 100644 (file)
@@ -505,8 +505,15 @@ struct stksess *stktable_lookup_key(struct stktable *t, struct stktable_key *key
        uint bucket;
        size_t len;
 
-       if (t->type == SMP_T_STR)
+       if (t->type == SMP_T_STR) {
                len = key->key_len + 1 < t->key_size ? key->key_len : t->key_size - 1;
+               /* the stored key is NUL-terminated, so all the other bucket
+                * computations stop at the first NUL. Do the same here or an
+                * embedded NUL would yield two different buckets for a same
+                * entry.
+                */
+               len = strnlen2(key->key, len);
+       }
        else
                len = t->key_size;
 
@@ -780,8 +787,11 @@ struct stksess *stktable_get_entry(struct stktable *table, struct stktable_key *
        if (!key)
                return NULL;
 
-       if (table->type == SMP_T_STR)
+       if (table->type == SMP_T_STR) {
                len = key->key_len + 1 < table->key_size ? key->key_len : table->key_size - 1;
+               /* see stktable_lookup_key() about the NUL truncation */
+               len = strnlen2(key->key, len);
+       }
        else
                len = table->key_size;