From 3d3f9f413869b949e48070b5bc593aa22cc2b8f2 Mon Sep 17 00:00:00 2001
From: Filippo Valsorda <filippo@golang.org>
Date: Sat, 10 Nov 2018 01:03:37 -0500
Subject: [PATCH] cryptobyte: don't ignore bytes added to BuilderContinuations
 of fixed-size Builders

Builders created with NewFixedBuilder were broken when used with
BuilderContinuations. The length of the bytes written to the
continuation would get added correctly to the parent, but the actual
content would be discarded.

For example, the BytesOrPanic() in TestFixedBuilderLengthPrefixed would
return [00 08] instead of [00 08 ff ff ff ff ff ff ff ff].

Change-Id: I80837a9bf3562751addcb827274649d9f52fc79a
Reviewed-on: https://go-review.googlesource.com/c/148882
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
---
 cryptobyte/builder.go         | 10 ++++++----
 cryptobyte/cryptobyte_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/cryptobyte/builder.go b/cryptobyte/builder.go
index d0c6c3f..ca7b1db 100644
--- a/cryptobyte/builder.go
+++ b/cryptobyte/builder.go
@@ -274,9 +274,11 @@ func (b *Builder) flushChild() {
 		return
 	}
 
-	if !b.fixedSize {
-		b.result = child.result // In case child reallocated result.
+	if b.fixedSize && &b.result[0] != &child.result[0] {
+		panic("cryptobyte: BuilderContinuation reallocated a fixed-size buffer")
 	}
+
+	b.result = child.result
 }
 
 func (b *Builder) add(bytes ...byte) {
@@ -284,7 +286,7 @@ func (b *Builder) add(bytes ...byte) {
 		return
 	}
 	if b.child != nil {
-		panic("attempted write while child is pending")
+		panic("cryptobyte: attempted write while child is pending")
 	}
 	if len(b.result)+len(bytes) < len(bytes) {
 		b.err = errors.New("cryptobyte: length overflow")
@@ -304,7 +306,7 @@ func (b *Builder) Unwrite(n int) {
 		return
 	}
 	if b.child != nil {
-		panic("attempted unwrite while child is pending")
+		panic("cryptobyte: attempted unwrite while child is pending")
 	}
 	length := len(b.result) - b.pendingLenLen - b.offset
 	if length < 0 {
diff --git a/cryptobyte/cryptobyte_test.go b/cryptobyte/cryptobyte_test.go
index b859cc9..fe658ef 100644
--- a/cryptobyte/cryptobyte_test.go
+++ b/cryptobyte/cryptobyte_test.go
@@ -412,6 +412,33 @@ func TestUnwrite(t *testing.T) {
 	})
 }
 
+func TestFixedBuilderLengthPrefixed(t *testing.T) {
+	bufCap := 10
+	inner := bytes.Repeat([]byte{0xff}, bufCap-2)
+	buf := make([]byte, 0, bufCap)
+	b := NewFixedBuilder(buf)
+	b.AddUint16LengthPrefixed(func(b *Builder) {
+		b.AddBytes(inner)
+	})
+	if got := b.BytesOrPanic(); len(got) != bufCap {
+		t.Errorf("Expected output lenght to be %d, got %d", bufCap, len(got))
+	}
+}
+
+func TestFixedBuilderPanicReallocate(t *testing.T) {
+	defer func() {
+		recover()
+	}()
+
+	b := NewFixedBuilder(make([]byte, 0, 10))
+	b1 := NewFixedBuilder(make([]byte, 0, 10))
+	b.AddUint16LengthPrefixed(func(b *Builder) {
+		*b = *b1
+	})
+
+	t.Error("Builder did not panic")
+}
+
 // ASN.1
 
 func TestASN1Int64(t *testing.T) {