From 0c36f8f86ab2a26567a3aaa79b3ddd78923e87e5 Mon Sep 17 00:00:00 2001 From: Oncilla Date: Sun, 5 May 2019 19:21:56 +0200 Subject: [PATCH] capnp: avoid panics on invalid input (#139) This PR prevents code from panicking and fixes some small errors during unpacking: Prevents malformed input for composite lists to cause the pogs library to panic. Make the `packed.Decoder` throw an error if the input is `0x00`. According to the spec, at least one more byte is required. Make the `packet.Decoder` throw an error if the input for the unpacked bytes (`0xFF` tag) does not contain enough unpacked bytes. Additionally, update the go/gazelle rules and update the bazel version on CI. Fixes #137 --- AUTHORS | 1 + CONTRIBUTORS | 1 + WORKSPACE | 10 +++++----- _travis/install.bash | 2 +- capn.go | 11 +++++++++-- internal/packed/packed.go | 23 +++++++++++++---------- internal/packed/packed_test.go | 21 +++++++++++++++++++++ pointer.go | 2 +- 8 files changed, 52 insertions(+), 19 deletions(-) diff --git a/AUTHORS b/AUTHORS index bd600be1..ce074947 100644 --- a/AUTHORS +++ b/AUTHORS @@ -12,6 +12,7 @@ CloudFlare Inc. Daniel Darabos +Dominik Roos Eran Duchan Evan Shaw Google Inc. diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 4a754150..018e2dd1 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -18,6 +18,7 @@ Alan Braithwaite Albert Strasheim Daniel Darabos +Dominik Roos Eran Duchan Evan Shaw Ian Denhardt diff --git a/WORKSPACE b/WORKSPACE index 7b5176a9..7d3d0a61 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -4,17 +4,17 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", - sha256 = "8b68d0630d63d95dacc0016c3bb4b76154fe34fca93efd65d1c366de3fcb4294", - urls = ["https://github.com/bazelbuild/rules_go/releases/download/0.12.1/rules_go-0.12.1.tar.gz"], + sha256 = "86ae934bd4c43b99893fc64be9d9fc684b81461581df7ea8fc291c816f5ee8c5", + urls = ["https://github.com/bazelbuild/rules_go/releases/download/0.18.3/rules_go-0.18.3.tar.gz"], ) http_archive( name = "bazel_gazelle", - sha256 = "ddedc7aaeb61f2654d7d7d4fd7940052ea992ccdb031b8f9797ed143ac7e8d43", - urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.12.0/bazel-gazelle-0.12.0.tar.gz"], + sha256 = "3c681998538231a2d24d0c07ed5a7658cb72bfb5fd4bf9911157c0e9ac6a2687", + urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.17.0/bazel-gazelle-0.17.0.tar.gz"], ) -load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies") +load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() diff --git a/_travis/install.bash b/_travis/install.bash index e0b01e5b..6870e3d1 100755 --- a/_travis/install.bash +++ b/_travis/install.bash @@ -13,7 +13,7 @@ die() { if [[ -z "$USE_BAZEL" || "$USE_BAZEL" -eq "0" ]]; then must go get -t ./... else - BAZEL_VERSION="${BAZEL_VERSION:-0.14.1}" + BAZEL_VERSION="${BAZEL_VERSION:-0.25.0}" case "$TRAVIS_OS_NAME" in linux) BAZEL_INSTALLER_URL="https://github.com/bazelbuild/bazel/releases/download/${BAZEL_VERSION}/bazel-${BAZEL_VERSION}-installer-linux-x86_64.sh" diff --git a/capn.go b/capn.go index 6de4c836..c4701e6e 100644 --- a/capn.go +++ b/capn.go @@ -200,6 +200,9 @@ func (s *Segment) readListPtr(base Address, val rawPointer) (List, error) { } sz := hdr.structSize() n := int32(hdr.offset()) + if n < 0 { + return List{}, errListSize + } // TODO(light): check that this has the same end address if tsize, ok := sz.totalSize().times(n); !ok { return List{}, errOverflow @@ -214,11 +217,15 @@ func (s *Segment) readListPtr(base Address, val rawPointer) (List, error) { flags: isCompositeList, }, nil } + n := val.numListElements() + if n < 0 { + return List{}, errListSize + } if lt == bit1List { return List{ seg: s, off: addr, - length: val.numListElements(), + length: n, flags: isBitList, }, nil } @@ -226,7 +233,7 @@ func (s *Segment) readListPtr(base Address, val rawPointer) (List, error) { seg: s, size: val.elementSize(), off: addr, - length: val.numListElements(), + length: n, }, nil } diff --git a/internal/packed/packed.go b/internal/packed/packed.go index 38573501..5cd2c120 100644 --- a/internal/packed/packed.go +++ b/internal/packed/packed.go @@ -142,6 +142,9 @@ func Unpack(dst, src []byte) ([]byte, error) { dst = allocWords(dst, int(src[0])) src = src[1:] n := copy(dst[start:], src) + if n < len(dst)-start { + return dst, io.ErrUnexpectedEOF + } src = src[n:] } } @@ -281,22 +284,22 @@ func (r *Reader) ReadWord(p []byte) error { switch tag { case zeroTag: z, err := r.rd.ReadByte() - if err == io.EOF { - r.err = io.ErrUnexpectedEOF - return nil - } else if err != nil { + if err != nil { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } r.err = err - return nil + return err } r.zeroes = int(z) case unpackedTag: l, err := r.rd.ReadByte() - if err == io.EOF { - r.err = io.ErrUnexpectedEOF - return nil - } else if err != nil { + if err != nil { + if err == io.EOF { + err = io.ErrUnexpectedEOF + } r.err = err - return nil + return err } r.literal = int(l) } diff --git a/internal/packed/packed_test.go b/internal/packed/packed_test.go index 4db535d2..3f8ab38b 100644 --- a/internal/packed/packed_test.go +++ b/internal/packed/packed_test.go @@ -222,6 +222,27 @@ var badDecompressionTests = []struct { name string input []byte }{ + { + "short zero tag", + []byte{0x00}, + }, + { + "short unpacked tag", + []byte{0xFF}, + }, + { + "unpacked tag, only one word", + []byte{ + 0xFF, 0, 0, 0, 0, 0, 0, 0, 0, + }, + }, + { + "unpacked tag, short unpacked word", + []byte{ + 0xFF, 0, 0, 0, 0, 0, 0, 0, 0, + 0x01, 0, + }, + }, { "wrong tag", []byte{ diff --git a/pointer.go b/pointer.go index 9b69e6b3..6aa7ea8d 100644 --- a/pointer.go +++ b/pointer.go @@ -155,7 +155,7 @@ func (p Ptr) text() (b []byte, ok bool) { // Text must be null-terminated. return nil, false } - return b[:len(b)-1 : len(b)], true + return b[: len(b)-1 : len(b)], true } // Data attempts to convert p into Data, returning nil if p is not a