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)
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;
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;