p2p: session.doRequest should define what happens if the response headers were more than the requested amount else encounter an integer overflow and a DOS vector

[odeke-em]

go-header/p2p/session.go

Line 232 in 18f0eb1

remainingHeaders := req.Amount - uint64(len(h))

looks very trusting hence suspicious because if the response headers were even off by just 1 extra, the value of remaining headers will overflow to uint64(-1) which is a very very large number. I suggest that we add a check to avoid this attack vector that could cause prepareRequests to consume large amounts of memory.

Kindly cc-ing @Wondertan @walldiss @liamsi

[walldiss]

Hey, thanks for catching that!

I took a closer look, and here's what's happening in the code: helpers.go link

var totalRespLn uint64
for i := 0; i < int(req.Amount); i++ {
    resp := new(p2p_pb.HeaderResponse)
    respLn, readErr := serde.Read(stream, resp)
    if readErr != nil {
        err = readErr
        break
    }

    totalRespLn += uint64(respLn)
    headers = append(headers, resp)
}

The condition len(h) > req.Amount shouldn't happen right now, but you're right—it could be error-prone. Adding an extra sanity check would be a good idea to prevent any potential issues