]> git.ipfire.org Git - thirdparty/wireguard-go.git/commitdiff
conn: windows: do not error out when receiving UDP jumbogram
authorJason A. Donenfeld <Jason@zx2c4.com>
Tue, 27 Apr 2021 02:07:03 +0000 (22:07 -0400)
committerJason A. Donenfeld <Jason@zx2c4.com>
Tue, 27 Apr 2021 02:07:03 +0000 (22:07 -0400)
If we receive a large UDP packet, don't return an error to receive.go,
which then terminates the receive loop. Instead, simply retry.

Considering Winsock's general finickiness, we might consider other
places where an attacker on the wire can generate error conditions like
this.

Reported-by: Sascha Dierberg <sascha.dierberg@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
conn/bind_windows.go

index fdd1c242474432ab0dd949f90b461eaeac9ee14d..42208b1767715055800a42862a0f95aca1c16e9f 100644 (file)
@@ -352,8 +352,11 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e
        }
        bind.rx.mu.Lock()
        defer bind.rx.mu.Unlock()
+
+       var err error
        var count uint32
        var results [1]winrio.Result
+retry:
        for tries := 0; count == 0 && tries < receiveSpins; tries++ {
                if tries > 0 {
                        if atomic.LoadUint32(isOpen) != 1 {
@@ -364,7 +367,7 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e
                count = winrio.DequeueCompletion(bind.rx.cq, results[:])
        }
        if count == 0 {
-               err := winrio.Notify(bind.rx.cq)
+               err = winrio.Notify(bind.rx.cq)
                if err != nil {
                        return 0, nil, err
                }
@@ -385,10 +388,19 @@ func (bind *afWinRingBind) Receive(buf []byte, isOpen *uint32) (int, Endpoint, e
                }
        }
        bind.rx.Return(1)
-       err := bind.InsertReceiveRequest()
+       err = bind.InsertReceiveRequest()
        if err != nil {
                return 0, nil, err
        }
+       // We limit the MTU well below the 65k max for practicality, but this means a remote host can still send us
+       // huge packets. Just try again when this happens. The infinite loop this could cause is still limited to
+       // attacker bandwidth, just like the rest of the receive path.
+       if windows.Errno(results[0].Status) == windows.WSAEMSGSIZE {
+               if atomic.LoadUint32(isOpen) != 1 {
+                       return 0, nil, net.ErrClosed
+               }
+               goto retry
+       }
        if results[0].Status != 0 {
                return 0, nil, windows.Errno(results[0].Status)
        }