From 8ae95fd8825161a5fdbefdd31befca8d45e00868 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Wed, 4 Oct 2017 13:44:34 +0100 Subject: [PATCH] crypto/tls: fix first byte test for 255 CBC padding bytes The BadCBCPadding255 test from bogo failed because at most 255 trailing bytes were checked, but for a padding of 255 there are 255 padding bytes plus 1 length byte with value 255. Change-Id: I7dd237c013d2c7c8599067246e31b7ba93106cf7 Reviewed-on: https://go-review.googlesource.com/68070 Reviewed-by: Adam Langley Run-TryBot: Adam Langley TryBot-Result: Gobot Gobot --- conn.go | 7 ++++--- conn_test.go | 12 ++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/conn.go b/conn.go index 9f32d4b..22017f5 100644 --- a/conn.go +++ b/conn.go @@ -213,10 +213,11 @@ func extractPadding(payload []byte) (toRemove int, good byte) { // if len(payload) >= (paddingLen - 1) then the MSB of t is zero good = byte(int32(^t) >> 31) - toCheck := 255 // the maximum possible padding length + // The maximum possible padding length plus the actual length field + toCheck := 256 // The length of the padded data is public, so we can use an if here - if toCheck+1 > len(payload) { - toCheck = len(payload) - 1 + if toCheck > len(payload) { + toCheck = len(payload) } for i := 0; i < toCheck; i++ { diff --git a/conn_test.go b/conn_test.go index e27c541..5c7f7ce 100644 --- a/conn_test.go +++ b/conn_test.go @@ -21,6 +21,12 @@ func TestRoundUp(t *testing.T) { } } +// will be initialized with {0, 255, 255, ..., 255} +var padding255Bad = [256]byte{} + +// will be initialized with {255, 255, 255, ..., 255} +var padding255Good = [256]byte{255} + var paddingTests = []struct { in []byte good bool @@ -36,9 +42,15 @@ var paddingTests = []struct { {[]byte{1, 4, 4, 4, 4, 4}, true, 1}, {[]byte{5, 5, 5, 5, 5, 5}, true, 0}, {[]byte{6, 6, 6, 6, 6, 6}, false, 0}, + {padding255Bad[:], false, 0}, + {padding255Good[:], true, 0}, } func TestRemovePadding(t *testing.T) { + for i := 1; i < len(padding255Bad); i++ { + padding255Bad[i] = 255 + padding255Good[i] = 255 + } for i, test := range paddingTests { paddingLen, good := extractPadding(test.in) expectedGood := byte(255)