search

corazawaf / coraza-proxy-wasm

proxy-wasm filter based on Coraza WAF
Apache License 2.0
100 stars 22 forks source link

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

Open iosetek opened 1 month ago

iosetek commented 1 month ago

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: https://github.com/corazawaf/coraza-proxy-wasm/blob/d1d6f28e419aead88005c088b01e5d3f04d69b70/wasmplugin/plugin.go#L432

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 https://github.com/corazawaf/coraza-proxy-wasm/pull/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 commented 1 month ago

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