Fix issue with http2 requests and captcha

[crycode-de]

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

[LaurenceJJones]

This wont directly fix the attached issue as that is being trigger by the read_body function (where we are returning our own custom error the http2 request need header) not by attempting to read the body in general (there was old lua module that used to throw an exception but this was 2 years ago which hopefully most distros have caught up by now). However, it always a good idea to make sure we are within a post request before attempting to verify a captcha.

[Copilot]

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 referer header to populate uri, which is later stored as previous_uri and used as the target of ngx.redirect without any validation. An attacker can trigger the captcha flow with a non-GET request and a crafted referer value 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 the referer header for redirect targets and ensure uri/previous_uri is restricted to same-site paths or validated against an allowlist of acceptable hosts before calling ngx.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.

[crycode-de]

This wont directly fix the attached issue as that is being trigger by the read_body function ...

As I figured out the issue (as last mentioned in #130 (comment)) is that the ngx.req.read_body() method gets called when captcha is configured and even if there is no body in the request (like in GET or HEAD requests).

lua-cs-bouncer/lib/crowdsec.lua

Lines 727 to 734 in 8bd0821

	if captcha_ok then
	-- if captcha can be used (configuration is valid)
	-- we check if the IP needs to validate its captcha before checking it against CrowdSec local API
	local previous_uri, flags = ngx.shared.crowdsec_cache:get("captcha_" .. ip)
	local source, state_id, err = flag.GetFlags(flags)
	if previous_uri ~= nil and state_id == flag.VERIFY_STATE then
	ngx.req.read_body()

Checking for a POST request will avoid the issue being triggered since it prevents ngx.req.read_body() from beeing called on other request types.
On POST requests the Content-Length header will be set by the browser and ngx.req.read_body() runs without errors.

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 captcha page is shown without any errors in the nginx logs and the captcha an be solved.

[neilbags]

I'm hitting the same issue, also on Ubuntu 24.04 and can confirm that this PR appears to fix it. It is still possible to send a POST request with no data to crash it but this is probably not a real-world issue.

This does seem like more of a workaround than a fix. As I understand it, this was a bug in lua-nginx-module that has already been fixed. The version in Ubuntu 24.04 is 0.10.26, which is supposed to include the fix, so this may be a packaging bug in Ubuntu, and we may be able to get this fixed there.

Is there any intention to merge this @LaurenceJJones?

[LaurenceJJones]

Is there any intention to merge this @LaurenceJJones?

I dont work at crowdsec anymore and dont have permissions to merge into the repo.