Skip to content
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

HTTP2 requests can bypass request payload scanning by adding request trailers #289

Open
iosetek opened this issue Aug 2, 2024 · 1 comment

Comments

@iosetek
Copy link

iosetek commented Aug 2, 2024

The Coraza Envoy WASM plugin contains a bug that allows bypassing request payload scanning for HTTP 2 requests with trailers

I've described it in the README.md located there https://github.com/iosetek/coraza-proxy-wasm/blob/fix-bypassing-request-body-scanning/demo/trailer_bypass_attack/README.md

The existing implementation triggers request payload scanning when end_of_stream parameter is set to true:

if endOfStream {

The thing is, that parameter will NOT be set to true for HTTP 2 requests with trailers. This is because end_of_stream is set to true only if no more payload chunks will arrive AND there will be no request trailers.

This scenario may be easily omitted, as it doesn't happen for HTTP 1 - even though HTTP standard allows adding request trailers, the Envoy team has decided to not trigger onRequestTrailers for HTTP 1 as most servers do not support trailers properly for HTTP 1.

To address the issue, I've raised a PR with solution proposal #288. It also contains the demo directory which is meant for reviewers to easily investigate the issue and see how the change affects the problem. The demo directory is not intended to be merged in the final stage.

@M4tteoP
Copy link
Member

M4tteoP commented Aug 4, 2024

I investigated a bit more the root cause.
OnHttpRequestBody current expected behaviour (in a simplified version) is the following:

  1. returns types.ActionPause as far as endOfStream is set to false, buffering all the body chunks.
  2. Once endOfStream is set to true, it means that the whole body has been received. So tx.ProcessRequestBody() is called, and if no interruption is raised, the stream is let continue via return types.ActionContinue.

The interesting behavior with a request that comes with trailers is that the return types.ActionPause is not honored, and the stream continues flowing by triggering OnHttpRequestTrailers.

It can be tested by just setting

func (ctx *httpContext) OnHttpRequestBody(bodySize int, endOfStream bool) types.Action {
	return types.ActionPause
	}

When a request with body and without trailers is sent, we get a stream timeout error, but when trailers come into play, the stream just flows.

That being said, the proposed workaround about relying on OnHttpRequestTrailers as another enforcement point before reaching upstream server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants