diff --git a/fuzz/oss-fuzz-build.sh b/fuzz/oss-fuzz-build.sh index d93a51085891c..ca202da6b06b6 100755 --- a/fuzz/oss-fuzz-build.sh +++ b/fuzz/oss-fuzz-build.sh @@ -23,6 +23,9 @@ prepare_teleport_api() { build_teleport_fuzzers() { + compile_native_go_fuzzer $TELEPORT_PREFIX/lib/srv/desktop/tdp \ + FuzzDecode fuzz_decode + compile_native_go_fuzzer $TELEPORT_PREFIX/lib/services \ FuzzParserEvalBoolPredicate fuzz_parser_eval_bool_predicate diff --git a/lib/srv/desktop/tdp/proto.go b/lib/srv/desktop/tdp/proto.go index 147d9e0a2d7bb..52f98bde3099b 100644 --- a/lib/srv/desktop/tdp/proto.go +++ b/lib/srv/desktop/tdp/proto.go @@ -229,6 +229,12 @@ func decodePNG2Frame(in byteReader) (PNG2Frame, error) { return PNG2Frame{}, trace.Wrap(err) } + // prevent allocation of giant buffers. + // this also avoids panic for pngLength ~ 4294967295 due to overflow. + if pngLength > maxPNGFrameDataLength { + return PNG2Frame{}, trace.BadParameter("pngLength too big: %v", pngLength) + } + // Allocate buffer that will fit PNG2Frame message // https://github.com/gravitational/teleport/blob/master/rfd/0037-desktop-access-protocol.md#27---png-frame-2 // message type (1) + png length (4) + left, right, top, bottom (4 x 4) + data => 21 + data @@ -663,12 +669,8 @@ func (s SharedDirectoryAnnounce) Encode() ([]byte, error) { } func decodeSharedDirectoryAnnounce(in io.Reader) (SharedDirectoryAnnounce, error) { - var completionID, directoryID uint32 - err := binary.Read(in, binary.BigEndian, &completionID) - if err != nil { - return SharedDirectoryAnnounce{}, trace.Wrap(err) - } - err = binary.Read(in, binary.BigEndian, &directoryID) + var directoryID uint32 + err := binary.Read(in, binary.BigEndian, &directoryID) if err != nil { return SharedDirectoryAnnounce{}, trace.Wrap(err) } @@ -1454,6 +1456,9 @@ const tdpMaxPathLength = 10240 const maxClipboardDataLength = 1024 * 1024 // 1MB const tdpMaxFileReadWriteLength = 1024 * 1024 // 1MB +// maxPNGFrameDataLength is maximum data length for PNG2Frame +const maxPNGFrameDataLength = 10 * 1024 * 1024 // 10MB + // These correspond to TdpErrCode enum in the rust RDP client. const ( ErrCodeNil uint32 = 0 diff --git a/lib/srv/desktop/tdp/proto_test.go b/lib/srv/desktop/tdp/proto_test.go index 51bc86282cf16..7f7b2119dc9eb 100644 --- a/lib/srv/desktop/tdp/proto_test.go +++ b/lib/srv/desktop/tdp/proto_test.go @@ -68,6 +68,50 @@ func TestEncodeDecode(t *testing.T) { } } +func FuzzDecode(f *testing.F) { + var corpus = []string{ + "0", + "\x02", + "\x1b\xff\xff\x800", + "\x1b\xff\xff\xff\xeb", + "\nn\x00\x00\x00\x04 {}", + "\v00000000\x00\x00\x00\x00", + "\nn\x00\x00\x00\x04 { }000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + } + + for _, s := range corpus { + f.Add([]byte(s)) + } + + f.Fuzz(func(t *testing.T, buf []byte) { + require.NotPanics(t, func() { + // decode random buffer + msg, err := Decode(buf) + if err != nil { + return + } + + // test that we can encode the message back: + buf2, err := msg.Encode() + require.NoError(t, err) + require.NotNil(t, buf2) + + // decode the new buffer. it must be equal to the original msg. + msg2, err := Decode(buf2) + require.NoError(t, err) + require.Equalf(t, msg, msg2, "mismatch for message %v", buf) + + // encode another time. + // after encoding, it must match the second buffer identically. + // this isn't the case for the first buffer, as there can be trailing bytes after the message. + buf3, err := msg2.Encode() + require.NoError(t, err) + require.NotNil(t, buf3) + require.Equal(t, buf2, buf3) + }) + }) +} + func TestBadDecode(t *testing.T) { // 254 is an unknown message type. _, err := Decode([]byte{254})