57 lines
2.4 KiB
Diff
57 lines
2.4 KiB
Diff
From 3e3b9eebf871510aee36c3a3336faac2f38c9559 Mon Sep 17 00:00:00 2001
|
|
From: Willy Tarreau <w@1wt.eu>
|
|
Date: Mon, 7 Apr 2025 15:30:43 +0200
|
|
Subject: [PATCH] BUG/MEDIUM: sample: fix risk of overflow when replacing
|
|
multiple regex back-refs
|
|
|
|
Aleandro Prudenzano of Doyensec and Edoardo Geraci of Codean Labs
|
|
reported a bug in sample_conv_regsub(), which can cause replacements
|
|
of multiple back-references to overflow the temporary trash buffer.
|
|
|
|
The problem happens when doing "regsub(match,replacement,g)": we're
|
|
replacing every occurrence of "match" with "replacement" in the input
|
|
sample, which requires a length check. For this, a max is applied, so
|
|
that a replacement may not use more than the remaining length in the
|
|
buffer. However, the length check is made on the replaced pattern and
|
|
not on the temporary buffer used to carry the new string. This results
|
|
in the remaining size to be usable for each input match, which can go
|
|
beyond the temporary buffer size if more than one occurrence has to be
|
|
replaced with something that's larger than the remaining room.
|
|
|
|
The fix proposed by Aleandro and Edoardo is the correct one (check on
|
|
"trash" not "output"), and is the one implemented in this patch.
|
|
|
|
While it is very unlikely that a config will replace multiple short
|
|
patterns each with a larger one in a request, this possibility cannot
|
|
be entirely ruled out (e.g. mask a known, short IP address using
|
|
"XXX.XXX.XXX.XXX"). However when this happens, the replacement pattern
|
|
will be static, and not be user-controlled, which is why this patch is
|
|
marked as medium.
|
|
|
|
The bug was introduced in 2.2 with commit 07e1e3c93e ("MINOR: sample:
|
|
regsub now supports backreferences"), so it must be backported to all
|
|
versions.
|
|
|
|
Special thanks go to Aleandro and Edoardo for reporting this bug with
|
|
a simple reproducer and a fix.
|
|
|
|
Conflict: NA
|
|
Reference: https://github.com/haproxy/haproxy/commit/3e3b9eebf871510aee36c3a3336faac2f38c9559
|
|
---
|
|
src/sample.c | 2 +-
|
|
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
|
|
diff --git a/src/sample.c b/src/sample.c
|
|
index 1e2ff7d2ee8e8..980c27cb6a507 100644
|
|
--- a/src/sample.c
|
|
+++ b/src/sample.c
|
|
@@ -3168,7 +3168,7 @@ static int sample_conv_regsub(const struct arg *arg_p, struct sample *smp, void
|
|
output->data = exp_replace(output->area, output->size, start, arg_p[1].data.str.area, pmatch);
|
|
|
|
/* replace the matching part */
|
|
- max = output->size - output->data;
|
|
+ max = trash->size - trash->data;
|
|
if (max) {
|
|
if (max > output->data)
|
|
max = output->data;
|