libp2p/go-buffer-pool

BufferPool.Get doesn't detect negative length values and blindly converts them into uint32(negative) which can trigger out of memory issues

Closed this issue · 0 comments

If I pass in a negative value whose uint32 overflown value fits within 31 bits, the code in (*BufferPool).Get or just .Get blindly converts it to a uint32 which wraps around to a very large positive number and causes massive memory allocations that'll cause out of memory errors to be triggered and the process will freeze and hopefully your OS will kill it but essentially it can freeze.

Repro

Here is the repro test that'll consume your RAM fast and freeze your other processes

var sink interface{}

func TestBufferPoolGetWithNegativeValue(t *testing.T) {
    for i := 0; i < 50; i++ {
        b := Get(-20008899456)
        sink = b
        Put(b)
    }
    if sink == nil { 
        t.Fatal("Test didn't run!")
    }   
    sink = (interface{})(nil)
} 

Remedy

The fix for this is rather trivial; when values are less than 0, please return nil ASAP

diff --git a/pool.go b/pool.go
index d812840..a83420e 100644
--- a/pool.go
+++ b/pool.go
@@ -55,7 +55,7 @@ type bufp struct {
 //
 // If no suitable buffer exists in the pool, Get creates one.
 func (p *BufferPool) Get(length int) []byte {
-       if length == 0 {
+       if length <= 0 {
                return nil
        }
        if length > MaxLength {

Suggestion

I kindly ask that if y'all have a bug bounty/security vulnerability reporting process that you paste something in your README.md so that I can report sensitive issues there. This work is courtesy of the Cosmos Network security team program to secure all dependencies. Thank you!