pauseBefore Bug

[wdahlenburg]

First off, amazing research on the CL.0/H2.0 smuggling techniques. The writeups and labs were great!

While I was going through the Server-side pause-based request smuggling lab, I found a frustrating bug that caused the pause position to be incorrectly placed.

This is how I wanted to specify the pause because of multiple \r\n\r\n sequences:

pausePoint = template.index('\r\n\r\n') + 4
engine.queue(template, pauseBefore=pausePoint, pauseTime=10000)

To reduce spoilers, let's say the pausePoint should have been at index 100 in the request characters. This should be the first occurrence of the \r\n\r\n in the request. I took a look at turbo-intruder after the fact and found that it was incorrectly setting the pause position to the pauseBefore value - 1. In this case it would have been 99.

However, specifying the \r\n\r\n with some arbitrary prefix to avoid pausing twice with the pauseMarker seemed to provide the correct pause position. It consistently would set the pause position to the correct value of 100.

engine.queue(template, pauseMarker=['d\r\n\r\n'], pauseTime=10000)

I was able to rearrange the smuggled headers so that the pauseMarker didn't exist in that portion of the request.

[albinowax]

Thanks for the report and PR. I'm not 100% sure what the right behaviour is here to be honest. The current, unpatched behaviour does match the argument name of pauseBefore - it pauses before sending the byte at the index specified. As you noted, this behaviour is a bit confusing when combined with string.index but I think it would be even more confusing if it was called 'pauseBefore' but actually paused after.

[wdahlenburg]

This made me audibly laugh out loud, not realizing the explicitness of the name. What about something likepauseAt, pausePoint, etc? In the negative sense I think there is still a miss on the naming, as -(CL) as the pauseBefore value would correctly set the position to 100.