66 lines
2.9 KiB
Diff
66 lines
2.9 KiB
Diff
|
From ed90ec01d0b2f26a07ff24cdb47b5cf9ed89b84e 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.
|
||
|
|
|
||
|
|
(cherry picked from commit 3e3b9eebf871510aee36c3a3336faac2f38c9559)
|
||
|
|
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
|
||
|
|
(cherry picked from commit db87c8d9fe621539531f6f915ba9e1755a2a26cb)
|
||
|
|
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
|
||
|
|
(cherry picked from commit ee1a64c2a04cc2cb38efb7e44f7ea7386d627bf6)
|
||
|
|
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
|
||
|
|
(cherry picked from commit 82c5355ce3a83344074e12f5e878dafae27efb96)
|
||
|
|
Signed-off-by: Aurelien DARRAGON <adarragon@haproxy.com>
|
||
|
|
|
||
|
|
Conflict: NA
|
||
|
|
Reference: https://github.com/haproxy/haproxy/commit/ed90ec01d0b2f26a07ff24cdb47b5cf9ed89b84e
|
||
|
|
---
|
||
|
|
src/sample.c | 2 +-
|
||
|
|
1 file changed, 1 insertion(+), 1 deletion(-)
|
||
|
|
|
||
|
|
diff --git a/src/sample.c b/src/sample.c
|
||
|
|
index 11c17528a9f0e..ff436f90a1747 100644
|
||
|
|
--- a/src/sample.c
|
||
|
|
+++ b/src/sample.c
|
||
|
|
@@ -2690,7 +2690,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;
|