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 :-/ Willy