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

DoS due to incorrect usage of http.Request Trailer #28

Open
fox-rose opened this issue Mar 26, 2024 · 0 comments
Open

DoS due to incorrect usage of http.Request Trailer #28

fox-rose opened this issue Mar 26, 2024 · 0 comments

Comments

@fox-rose
Copy link

This issue
occurs when client requests utilize the HTTP Trailer feature, which essentially represents a
second set of headers sent after the body. The vulnerability manifests if the map holding
these trailer headers is not initialized prior to use.

One can pertinently note that a reference implementation utilizing the ohttp-go library wraps
the entire library into the Golang standard http library, which has a built-in mechanism to
recover from raised panics. However, any application without this feature in place will panic
and crash.

Affected file:
bhttp.go:240
Affected code:
// Construct the raw request
request, err := http.NewRequest(controlData.method, url.String(),
bytes.NewBuffer(content))
if err != nil {
return nil, err
}
for _, field := range headerFields.fields {
request.Header.Set(field.name, field.value)
}
for _, field := range trailerFields.fields {
request.Trailer.Set(field.name, field.value)
}
The following test function will trigger the crash by providing a binary request with a nonzero
Trailer:
PoC:
func TestHttpTrailerCrash(t *testing.T) {
data := []byte{0x03, 0x04, 0x50, 0x4f, 0x53, 0x54, 0x03, 0x6d, 0x30,
0x30, 0x00, 0x00,

0x00, 0x03, 0x30, 0x30, 0x30, 0x30, 0x06}
// Tries to set trailer values without first initializing trailer to
a map, so it's a nil dereference.
ohttp.UnmarshalBinaryRequest(data)
}

To mitigate this issue, we suggest improving and standardizing error handling across
all functions in the library. Implementing a uniform return pattern for functions, specifically in
the form of (result, error), will streamline error reporting and handling, thereby enhancing the
robustness of the application. Moreover, incorporating fuzz testing into the CI/CD pipeline is
recommended. This proactive approach will aid in early detection of vulnerabilities,
substantially bolstering the security of the ohttp-go library.

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

1 participant