Validate received chunks before conversion to indexes

https://github.com/anacrolix/torrent/issues/791

diff --git a/peerconn.go b/peerconn.go
index 7e872d2c21aeb5a94f3013f122af47dfd8467588..5ed5bde69eff88dcf9fe9e16b682bba7bc80c1b4 100644 (file)
--- a/peerconn.go
+++ b/peerconn.go
@@ -596,6 +596,10 @@ type messageWriter func(pp.Message) bool
 // This function seems to only used by Peer.request. It's all logic checks, so maybe we can no-op it
 // when we want to go fast.
 func (cn *Peer) shouldRequest(r RequestIndex) error {
+       err := cn.t.checkValidReceiveChunk(cn.t.requestIndexToRequest(r))
+       if err != nil {
+               return err
+       }
        pi := cn.t.pieceIndexOfRequestIndex(r)
        if cn.requestState.Cancelled.Contains(r) {
                return errors.New("request is cancelled and waiting acknowledgement")
@@ -1425,8 +1429,13 @@ func (c *Peer) receiveChunk(msg *pp.Message) error {
        chunksReceived.Add("total", 1)
 
        ppReq := newRequestFromMessage(msg)
-       req := c.t.requestIndexFromRequest(ppReq)
        t := c.t
+       err := t.checkValidReceiveChunk(ppReq)
+       if err != nil {
+               err = log.WithLevel(log.Warning, err)
+               return err
+       }
+       req := c.t.requestIndexFromRequest(ppReq)
 
        if c.bannableAddr.Ok {
                t.smartBanCache.RecordBlock(c.bannableAddr.Value, req, msg.Piece)
@@ -1508,7 +1517,7 @@ func (c *Peer) receiveChunk(msg *pp.Message) error {
                p.cancel(req)
        }
 
-       err := func() error {
+       err = func() error {
                cl.unlock()
                defer cl.lock()
                concurrentChunkWrites.Add(1)

diff --git a/torrent.go b/torrent.go
index 1b2fc978f45c0bc50168fdb3a96775504cb98b14..e0bc7246320ca7880dbee91bb06c685e03f5d519 100644 (file)
--- a/torrent.go
+++ b/torrent.go
@@ -2541,3 +2541,22 @@ type requestState struct {
        peer *Peer
        when time.Time
 }
+
+// Returns an error if a received chunk is out of bounds in someway.
+func (t *Torrent) checkValidReceiveChunk(r Request) error {
+       if !t.haveInfo() {
+               return errors.New("torrent missing info")
+       }
+       if int(r.Index) >= t.numPieces() {
+               return fmt.Errorf("chunk index %v, torrent num pieces %v", r.Index, t.numPieces())
+       }
+       pieceLength := t.pieceLength(pieceIndex(r.Index))
+       if r.Begin >= pieceLength {
+               return fmt.Errorf("chunk begins beyond end of piece (%v >= %v)", r.Begin, pieceLength)
+       }
+       // We could check chunk lengths here, but chunk request size is not changed often, and tricky
+       // for peers to manipulate as they need to send potentially large buffers to begin with. There
+       // should be considerable checks elsewhere for this case due to the network overhead. We should
+       // catch most of the overflow manipulation stuff by checking index and begin above.
+       return nil
+}