Re: [PATCH] BUG/MINOR: sample: Fix adjusting size in field converter

2021-04-13 Thread Willy Tarreau
Hi Thayne,

On Sun, Apr 11, 2021 at 11:26:59PM -0600, Thayne McCombs wrote:
> Adjust the size of the sample buffer before we change the "area"
> pointer. The change in size is calculated as the difference between the
> original pointer and the new start pointer. But since the
> `smp->data.u.str.area` assignment results in `smp->data.u.str.area` and
> `start` being the same pointer, we always ended up substracting zero.
> This changes it to change the size by the actual amount it changed.
> 
> I'm not entirely sure what the impact of this is, but the previous code
> seemed wrong.

So I carefully reviewed it, and not only you're totally right, but I
could figure in which case it is harmful. All accesses limit themselves
to the amount of data except one, the binary key padding for a stick
table. So it is technically possible to use it to write zeroes past the
end of the string in such a construct where  is of type binary
with keys at least as large as your buffers (lots of 'if') :

  hdr(foo),field(2,:),in_table(table)

Thus I tagged it "MEDIUM" in the end.

Thank you!
Willy



[PATCH] BUG/MINOR: sample: Fix adjusting size in field converter

2021-04-11 Thread Thayne McCombs
Adjust the size of the sample buffer before we change the "area"
pointer. The change in size is calculated as the difference between the
original pointer and the new start pointer. But since the
`smp->data.u.str.area` assignment results in `smp->data.u.str.area` and
`start` being the same pointer, we always ended up substracting zero.
This changes it to change the size by the actual amount it changed.

I'm not entirely sure what the impact of this is, but the previous code
seemed wrong.
---
 src/sample.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/sample.c b/src/sample.c
index eac489538..05d78bcb9 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -2561,13 +2561,13 @@ static int sample_conv_field(const struct arg *arg_p, 
struct sample *smp, void *
if (!smp->data.u.str.data)
return 1;
 
-   smp->data.u.str.area = start;
-
/* Compute remaining size if needed
Note: smp->data.u.str.size cannot be set to 0 */
if (smp->data.u.str.size)
smp->data.u.str.size -= start - smp->data.u.str.area;
 
+   smp->data.u.str.area = start;
+
return 1;
 }
 
-- 
2.31.1