[GH-ISSUE #178] device: RandomTrailers panics the process on every cookie reply, and never appends a trailer #85

New issue

Closed

opened 2026-10-13 21:03:26 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-13 21:03:26 +00:00

Owner

Originally created by @AYastrebov on GitHub (Aug 13, 2026).
Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/178

Originally assigned to: @ygurov on GitHub.

Affected upstream source

Repository: https://github.com/amnezia-vpn/amneziawg-go
Version: v3.1.20260813
Immutable revision: 08271d00b330999cde4ec41edd067e7e1b1d9e89
At investigation time, upstream master was identical to that revision.
Introduced by 1f50ad7 ("feat: awg 3.1 features").

Summary

SendHandshakeCookie passes the trailer length as the slice capacity instead
of adding it to the length:

github.com/amnezia-vpn/amneziawg-go@08271d00b3/device/send.go (L266)

padding := int(device.paddings.cookie.Load())
trailerLen := max(device.randomTrailer(padding+MessageCookieReplySize), 0)

buf := make([]byte, padding+MessageCookieReplySize, trailerLen)   // len, cap

The initiation and response paths add it to the length instead:

buf := make([]byte, padding+MessageInitiationSize+trailerLen)   // SendHandshakeInitiation
buf := make([]byte, padding+MessageResponseSize+trailerLen)     // SendHandshakeResponse

make([]byte, len, cap) panics when cap is below len. So if trailerLen
is under padding+64, the call panics. RoutineHandshake has no recover(),
and neither does anything else in device/, so that takes down the process.

If trailerLen is padding+64 or more there is no panic, but buf still has
length padding+64. That makes buf[padding+MessageCookieReplySize:] empty,
the rand.Read(trailer) on the next line writes nothing, and the datagram
goes out at its normal size.

Every reachable value of trailerLen lands in one branch or the other, so on
this path the trailer is never actually sent.

Safe reproduction

No keys, addresses or configuration involved. This is the allocation on its
own, with the same expression and the same inputs the caller supplies.
randomTrailer returns fastrandn(DefaultUdpWindow - packetSize), so with
DefaultUdpWindow = 500 and packetSize = padding + MessageCookieReplySize
the reachable values of trailerLen are [0, 500-(padding+64)).

package main

import "fmt"

const MessageCookieReplySize = 64

func attempt(padding, trailerLen int) (panicked bool, trailer int) {
	defer func() {
		if r := recover(); r != nil {
			panicked = true
		}
	}()
	buf := make([]byte, padding+MessageCookieReplySize, trailerLen)
	return false, len(buf[padding+MessageCookieReplySize:])
}

func main() {
	for _, padding := range []int{0, 16} {
		panics, empty := 0, 0
		max := 500 - (padding + MessageCookieReplySize) // DefaultUdpWindow
		for t := 0; t < max; t++ {
			if p, tr := attempt(padding, t); p {
				panics++
			} else if tr == 0 {
				empty++
			}
		}
		fmt.Printf("S3=%-3d draws=%-4d panics=%-4d (%2.0f%%)  empty-trailer=%d\n",
			padding, max, panics, 100*float64(panics)/float64(max), empty)
	}
}

Observed on go1.26.5:

S3=0   draws=436  panics=64   (15%)  empty-trailer=372
S3=16  draws=420  panics=80   (19%)  empty-trailer=340

The panic is panic: runtime error: makeslice: cap out of range.

Reachability

SendHandshakeCookie runs in RoutineHandshake when device.IsUnderLoad()
is true and the initiation's MAC2 does not verify (device/receive.go:377).
That is the ordinary state during a handshake flood, which is what cookies
exist to answer in the first place.

Reaching it needs a valid MAC1, so the sender has to know the responder's
static public key. On a VPN server that is not secret. Nothing else is
required, no handshake has to complete, and the panic ends the process rather
than dropping one peer.

This only affects devices with RandomTrailers enabled. When it is off,
randomTrailer returns -1 and max(-1, 0) gives a trailerLen of 0, which
panics as well, so what matters is whether the feature is on rather than which
value gets drawn.

Suggested fix

-	buf := make([]byte, padding+MessageCookieReplySize, trailerLen)
+	buf := make([]byte, padding+MessageCookieReplySize+trailerLen)

That lines up with the initiation and response paths and gives
buf[padding+MessageCookieReplySize:] the trailer those two already write
into.

Notes

Found while implementing AmneziaWG 3.1 in a Rust port that follows this device
as its reference. Happy to open a PR for the one-line change if it helps.

Originally created by @AYastrebov on GitHub (Aug 13, 2026). Original GitHub issue: https://github.com/amnezia-vpn/amneziawg-go/issues/178 Originally assigned to: @ygurov on GitHub. ## Affected upstream source - Repository: `https://github.com/amnezia-vpn/amneziawg-go` - Version: `v3.1.20260813` - Immutable revision: `08271d00b330999cde4ec41edd067e7e1b1d9e89` - At investigation time, upstream `master` was identical to that revision. - Introduced by `1f50ad7` ("feat: awg 3.1 features"). ## Summary `SendHandshakeCookie` passes the trailer length as the slice capacity instead of adding it to the length: https://github.com/amnezia-vpn/amneziawg-go/blob/08271d00b330999cde4ec41edd067e7e1b1d9e89/device/send.go#L266 ```go padding := int(device.paddings.cookie.Load()) trailerLen := max(device.randomTrailer(padding+MessageCookieReplySize), 0) buf := make([]byte, padding+MessageCookieReplySize, trailerLen) // len, cap ``` The initiation and response paths add it to the length instead: ```go buf := make([]byte, padding+MessageInitiationSize+trailerLen) // SendHandshakeInitiation buf := make([]byte, padding+MessageResponseSize+trailerLen) // SendHandshakeResponse ``` `make([]byte, len, cap)` panics when `cap` is below `len`. So if `trailerLen` is under `padding+64`, the call panics. `RoutineHandshake` has no `recover()`, and neither does anything else in `device/`, so that takes down the process. If `trailerLen` is `padding+64` or more there is no panic, but `buf` still has length `padding+64`. That makes `buf[padding+MessageCookieReplySize:]` empty, the `rand.Read(trailer)` on the next line writes nothing, and the datagram goes out at its normal size. Every reachable value of `trailerLen` lands in one branch or the other, so on this path the trailer is never actually sent. ## Safe reproduction No keys, addresses or configuration involved. This is the allocation on its own, with the same expression and the same inputs the caller supplies. `randomTrailer` returns `fastrandn(DefaultUdpWindow - packetSize)`, so with `DefaultUdpWindow = 500` and `packetSize = padding + MessageCookieReplySize` the reachable values of `trailerLen` are `[0, 500-(padding+64))`. ```go package main import "fmt" const MessageCookieReplySize = 64 func attempt(padding, trailerLen int) (panicked bool, trailer int) { defer func() { if r := recover(); r != nil { panicked = true } }() buf := make([]byte, padding+MessageCookieReplySize, trailerLen) return false, len(buf[padding+MessageCookieReplySize:]) } func main() { for _, padding := range []int{0, 16} { panics, empty := 0, 0 max := 500 - (padding + MessageCookieReplySize) // DefaultUdpWindow for t := 0; t < max; t++ { if p, tr := attempt(padding, t); p { panics++ } else if tr == 0 { empty++ } } fmt.Printf("S3=%-3d draws=%-4d panics=%-4d (%2.0f%%) empty-trailer=%d\n", padding, max, panics, 100*float64(panics)/float64(max), empty) } } ``` Observed on `go1.26.5`: ``` S3=0 draws=436 panics=64 (15%) empty-trailer=372 S3=16 draws=420 panics=80 (19%) empty-trailer=340 ``` The panic is `panic: runtime error: makeslice: cap out of range`. ## Reachability `SendHandshakeCookie` runs in `RoutineHandshake` when `device.IsUnderLoad()` is true and the initiation's MAC2 does not verify (`device/receive.go:377`). That is the ordinary state during a handshake flood, which is what cookies exist to answer in the first place. Reaching it needs a valid MAC1, so the sender has to know the responder's static public key. On a VPN server that is not secret. Nothing else is required, no handshake has to complete, and the panic ends the process rather than dropping one peer. This only affects devices with `RandomTrailers` enabled. When it is off, `randomTrailer` returns `-1` and `max(-1, 0)` gives a `trailerLen` of 0, which panics as well, so what matters is whether the feature is on rather than which value gets drawn. ## Suggested fix ```diff - buf := make([]byte, padding+MessageCookieReplySize, trailerLen) + buf := make([]byte, padding+MessageCookieReplySize+trailerLen) ``` That lines up with the initiation and response paths and gives `buf[padding+MessageCookieReplySize:]` the trailer those two already write into. ## Notes Found while implementing AmneziaWG 3.1 in a Rust port that follows this device as its reference. Happy to open a PR for the one-line change if it helps.

bsakaguchi closed this issue

2026-10-13 21:03:26 +00:00

bsakaguchi commented

2026-10-13 21:03:28 +00:00

Author

Owner

@ygurov commented on GitHub (Aug 13, 2026):

Hi @AYastrebov,
Oh wow, must be the typo. Appreciate for the quick finding!
Let me make a hotfix

@ygurov commented on GitHub (Aug 13, 2026): Hi @AYastrebov, Oh wow, must be the typo. Appreciate for the quick finding! Let me make a hotfix

bsakaguchi commented

2026-10-13 21:03:28 +00:00

Author

Owner

@ygurov commented on GitHub (Aug 13, 2026):

Fixed by github.com/amnezia-vpn/amneziawg-go@1b86b2ae0e, introduced v3.1.20260814

@ygurov commented on GitHub (Aug 13, 2026): Fixed by https://github.com/amnezia-vpn/amneziawg-go/commit/1b86b2ae0e493e7ea93f8c1a0f0cb6735b1551f1, introduced v3.1.20260814

bsakaguchi referenced this issue

2026-10-13 21:03:58 +00:00

[PR #85] [MERGED] fix: restore Dockerfile changes #138