Conversation
|
Can confirm both issues are real - checked against our fork (damnurmum/OpenFlux-Android), which carries The unbounded LZ4 |
- Add bounds check in rawsocket readLoop/WritePackets to prevent slice bounds panic when IHL field exceeds actual packet length. - Add io.LimitReader to LZ4 decompression to prevent OOM crash from malicious decompression bomb payload.
bd20c0a to
a10ef51
Compare
|
The Darwin raw socket code got removed upstream anyway, so the fix there is no longer needed. @damnurmum glad to hear the LZ4 limit is useful for your fork as well |
While testing the tunnel stability under heavy load, I noticed a scenario where the exit node process crashed unexpectedly due to a slice bounds panic in the raw socket
readLoop. The code was calculatingipHeaderLenfrom the first byte of the IP packet, but if a truncated or malformed packet arrives where this length exceeds the actual bytes received, slicingpktCopy[ipHeaderLen:]panics instantly. Because this loop isn't wrapped inSafeGo, the panic takes down the entire tunnel.Added a quick bounds check before the slice operation in both Linux and Darwin raw socket endpoints to fix that. If
ipHeaderLen + 20is larger than the packet size, the code just skips it now.Another issue caught my attention in
compressor.goregarding how LZ4 decompression is handled. The current implementation uses an unboundedio.ReadAllon the LZ4 reader, which means a tiny crafted compressed payload could decompress into gigabytes of zeros. That would easily cause an OOM crash on either the client or the exit node, so capping it withio.LimitReaderlimits the output to a safe 10MB to prevent any decompression bomb issues.