-
Notifications
You must be signed in to change notification settings - Fork 16
Fix issue with http2 requests and captcha #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The lua-nginx-module requires the content-length header to be set on http2 requests when ngx.req.read_body() is called and throws an error otherwise. This fix checks for POST request method before reading the body to avoid the error on other request methods. Ref crowdsecurity#130
|
This wont directly fix the attached issue as that is being trigger by the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue where HTTP/2 requests would cause errors in the captcha verification flow. The lua-nginx-module requires a content-length header when reading request bodies on HTTP/2 requests, but non-POST methods (GET, HEAD, OPTIONS, etc.) typically don't have bodies or content-length headers, causing the module to throw errors.
Changes:
- Added POST method check before reading request body in captcha verification
- Removed trailing whitespace on line 792
Comments suppressed due to low confidence (1)
lib/crowdsec.lua:803
- The code uses the user-controlled
refererheader to populateuri, which is later stored asprevious_uriand used as the target ofngx.redirectwithout any validation. An attacker can trigger the captcha flow with a non-GET request and a craftedreferervalue pointing to an external domain, then, after the captcha is solved, the client will be redirected to that attacker-controlled URL, enabling open-redirect based phishing or token-stealing attacks. To mitigate this, avoid trusting therefererheader for redirect targets and ensureuri/previous_uriis restricted to same-site paths or validated against an allowlist of acceptable hosts before callingngx.redirect.
if ngx.req.get_method() ~= "GET" then
local headers, err = ngx.req.get_headers()
for k, v in pairs(headers) do
if k == "referer" then
uri = v
end
end
end
local succ, err, forcible = ngx.shared.crowdsec_cache:set("captcha_"..ip, uri , 60, bit.bor(flag.VERIFY_STATE, remediationSource))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
As I figured out the issue (as last mentioned in #130 (comment)) is that the lua-cs-bouncer/lib/crowdsec.lua Lines 727 to 734 in 8bd0821
Checking for a POST request will avoid the issue being triggered since it prevents Should be the linked issue, or am i missing something? Tested on Ubuntu 24.04 with nginx 1.24.0 and libnginx-mod-http-lua 1:0.10.26-2. |
The lua-nginx-module requires the content-length header to be set on http2 requests when
ngx.req.read_body()is called and throws an error otherwise. This fix checks for POST request method before reading the body to avoid the error on other request methods.Ref #130