Le 04/10/2017 à 05:12, Willy Tarreau a écrit :
On Wed, Oct 04, 2017 at 05:07:07AM +0200, Willy Tarreau wrote:
On Wed, Oct 04, 2017 at 04:40:53AM +0200, Willy Tarreau wrote:
On Tue, Oct 03, 2017 at 06:57:45PM +0200, Marcus Ulbrich wrote:
Hey Jarno,
This seems to work stable!
The idea for this acl was to prevent attackers testing for example MySQL injection
by sleep command. ,,sleep" is in none of our URLs!
Do you have an idea about an acl about this not crashing haproxy?
I wouldn't be surprized if there was an issue for example with a %00 causing
a length mismatch between two parts. At least now we have an idea where to
look for the bug. It shouldn't take very long anymore to spot the problem.
I'm seeing an issue in the url_dec sample converter :
static int sample_conv_url_dec(const struct arg *args, struct sample *smp,
void
*private)
{
/* If the constant flag is set or if not size is avalaible at
* the end of the buffer, copy the string in other buffer
* before decoding.
*/
if (smp->flags & SMP_F_CONST || smp->data.u.str.size <=
smp->data.u.str.
len) {
struct chunk *str = get_trash_chunk();
memcpy(str->str, smp->data.u.str.str, smp->data.u.str.len);
Here it's missing a check to ensure that str.len <= str.size. Normally there
is no known case where this could happen, but the preferred smp_dup() performs
the check and truncates the input if needed. I don't know if there's a reason
for this to ever happen. Hmmm in fact there's one theorical case where it could
happen, it's if the sample contains a full buffer (str.len == str.size), and
then we can overflow the block by one byte when writing the trailing zero. But
it's not possible in HTTP since the request is at least a few bytes long, and
there's the reserve.
smp->data.u.str.str = str->str;
smp->data.u.str.size = str->size;
smp->flags &= ~SMP_F_CONST;
}
So if you're interested, you can replace the block above by this :
if (smp->flags & SMP_F_CONST || smp->data.u.str.size <=
smp->data.u.str.
len)
smp_dup(smp);
If this stops crashing we'll have to try to figure in which situation the
case above may happen :-/
By the way if you manage to print the contents of "smp" in
sample_conv_str2lower() it could help, at least to find smp->str.{len,size}.
Hi,
I found a bug in url_dec sample converter that could lead to a
segmentation fault. the function sample_conv_url_dec always succeeds
even if url_decode failed. And the return value of url_decode is always
used to set the decoded string length. url_decode returns -1 when an
invalid character is found. So, when we try to decode an invalid URL, we
end up to manipulate an invalid sample, leading to undefined behavior.
I fixed the bug but I'm going to send the patch separately to this
thread to be sure everyone see it.
--
Christopher Faulet
