search

karlseguin / http.zig

An HTTP/1.1 server for zig
MIT License
454 stars 31 forks source link

Bad client content-length can cause runtime panic #16

Closed zigster64 closed 11 months ago

zigster64 commented 11 months ago

Seeing this panic trace in the application logs

image

Dont have proof or solution yet ... but trying this to see if it catches some bad behaviour at the client end

https://github.com/karlseguin/http.zig/commit/40be0f090e5f3b25643da678e508c2b38d0d1356

Looks like there may be some edge cases where either header_overread is not initialised ... OR ... client is deliberately sending extra body data that doesnt match what they claim to be sending in content-length, creating an integer overflow panic.

Will keep investigating and see what I can find.

karlseguin commented 11 months ago

This scenario is easy to replicate in a test. Just a matter of putting more data on the socket than content-length indicates. In such cases, I think it's reasonable/correct to close the connection as things are rather ambiguous. https://github.com/karlseguin/http.zig/commit/608ffe5705d0fc5e2e984bf3ddcd054ac1b12ef0 deals with it.

Malicious client aside, I'm trying to figure out why this can happen and whether it should be handled differently.

zigster64 commented 11 months ago

Closing this as fixed now

good work