nbt/encoding.go: Allow strings up to max uint16 length. This also fix…

…es a potential attack factor for NBT with negative string lengths and provides better error message for invalid strings.

minecraft/nbt/encoding.go

@@ -86,8 +86,8 @@ func (networkLittleEndian) WriteInt64(w *offsetWriter, x int64) error {
 
 // WriteString ...
 func (networkLittleEndian) WriteString(w *offsetWriter, x string) error {
-	if len(x) > math.MaxInt16 {
-		return InvalidStringError{Off: w.off, String: x, Err: errors.New("string length exceeds maximum length prefix")}
+	if len(x) > math.MaxUint16 {
+		return InvalidStringError{Off: w.off, N: uint(len(x)), Err: errors.New("string length exceeds maximum length prefix")}
 	}
 	ux := uint32(len(x))
 	for ux >= 0x80 {
@@ -159,8 +159,8 @@ func (e networkLittleEndian) String(r *offsetReader) (string, error) {
 			break
 		}
 	}
-	if length > math.MaxInt16 {
-		return "", InvalidStringError{Off: r.off, Err: errors.New("string length exceeds maximum length prefix")}
+	if length > math.MaxUint16 {
+		return "", InvalidStringError{N: uint(length), Off: r.off, Err: errors.New("string length exceeds maximum length prefix")}
 	}
 	data := make([]byte, length)
 	if _, err := r.Read(data); err != nil {

minecraft/nbt/encoding_big_endian.go

@@ -58,10 +58,10 @@ func (bigEndian) WriteFloat64(w *offsetWriter, x float64) error {
 
 // WriteString ...
 func (e bigEndian) WriteString(w *offsetWriter, x string) error {
-	if len(x) > math.MaxInt16 {
-		return InvalidStringError{Off: w.off, String: x, Err: errors.New("string length exceeds maximum length prefix")}
+	if len(x) > math.MaxUint16 {
+		return InvalidStringError{Off: w.off, N: uint(len(x)), Err: errors.New("string length exceeds maximum length prefix")}
 	}
-	if err := e.WriteInt16(w, int16(len(x))); err != nil {
+	if err := e.WriteInt16(w, int16(uint16(len(x)))); err != nil {
 		return FailedWriteError{Op: "WriteString", Off: w.off}
 	}
 	// Use unsafe conversion from a string to a byte slice to prevent copying.
@@ -123,7 +123,7 @@ func (e bigEndian) String(r *offsetReader) (string, error) {
 	if err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}
-	b := make([]byte, n)
+	b := make([]byte, uint16(n))
 	if _, err := r.Read(b); err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}
@@ -216,10 +216,10 @@ func (littleEndian) WriteFloat64(w *offsetWriter, x float64) error {
 
 // WriteString ...
 func (e littleEndian) WriteString(w *offsetWriter, x string) error {
-	if len(x) > math.MaxInt16 {
-		return InvalidStringError{Off: w.off, String: x, Err: errors.New("string length exceeds maximum length prefix")}
+	if len(x) > math.MaxUint16 {
+		return InvalidStringError{Off: w.off, N: uint(len(x)), Err: errors.New("string length exceeds maximum length prefix")}
 	}
-	if err := e.WriteInt16(w, int16(len(x))); err != nil {
+	if err := e.WriteInt16(w, int16(uint16(len(x)))); err != nil {
 		return FailedWriteError{Op: "WriteString", Off: w.off}
 	}
 	// Use unsafe conversion from a string to a byte slice to prevent copying.
@@ -281,7 +281,7 @@ func (e littleEndian) String(r *offsetReader) (string, error) {
 	if err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}
-	b := make([]byte, strLen)
+	b := make([]byte, uint16(strLen))
 	if _, err := r.Read(b); err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}

minecraft/nbt/encoding_little_endian.go

@@ -58,10 +58,10 @@ func (littleEndian) WriteFloat64(w *offsetWriter, x float64) error {
 
 // WriteString ...
 func (e littleEndian) WriteString(w *offsetWriter, x string) error {
-	if len(x) > math.MaxInt16 {
-		return InvalidStringError{Off: w.off, String: x, Err: errors.New("string length exceeds maximum length prefix")}
+	if len(x) > math.MaxUint16 {
+		return InvalidStringError{Off: w.off, N: uint(len(x)), Err: errors.New("string length exceeds maximum length prefix")}
 	}
-	if err := e.WriteInt16(w, int16(len(x))); err != nil {
+	if err := e.WriteInt16(w, int16(uint16(len(x)))); err != nil {
 		return FailedWriteError{Op: "WriteString", Off: w.off}
 	}
 	// Use unsafe conversion from a string to a byte slice to prevent copying.
@@ -123,7 +123,7 @@ func (e littleEndian) String(r *offsetReader) (string, error) {
 	if err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}
-	b := make([]byte, strLen)
+	b := make([]byte, uint16(strLen))
 	if _, err := r.Read(b); err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}
@@ -216,10 +216,10 @@ func (bigEndian) WriteFloat64(w *offsetWriter, x float64) error {
 
 // WriteString ...
 func (e bigEndian) WriteString(w *offsetWriter, x string) error {
-	if len(x) > math.MaxInt16 {
-		return InvalidStringError{Off: w.off, String: x, Err: errors.New("string length exceeds maximum length prefix")}
+	if len(x) > math.MaxUint16 {
+		return InvalidStringError{Off: w.off, N: uint(len(x)), Err: errors.New("string length exceeds maximum length prefix")}
 	}
-	if err := e.WriteInt16(w, int16(len(x))); err != nil {
+	if err := e.WriteInt16(w, int16(uint16(len(x)))); err != nil {
 		return FailedWriteError{Op: "WriteString", Off: w.off}
 	}
 	// Use unsafe conversion from a string to a byte slice to prevent copying.
@@ -281,7 +281,7 @@ func (e bigEndian) String(r *offsetReader) (string, error) {
 	if err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}
-	b := make([]byte, strLen)
+	b := make([]byte, uint16(strLen))
 	if _, err := r.Read(b); err != nil {
 		return "", BufferOverrunError{Op: "String"}
 	}

minecraft/nbt/err.go

@@ -120,14 +120,14 @@ func (err IncompatibleTypeError) Error() string {
 // InvalidStringError is returned if a string read is not valid, meaning it does not exist exclusively out of
 // utf8 characters, or if it is longer than the length prefix can carry.
 type InvalidStringError struct {
-	Off    int64
-	Err    error
-	String string
+	Off int64
+	Err error
+	N   uint
 }
 
 // Error ...
 func (err InvalidStringError) Error() string {
-	return fmt.Sprintf("nbt: string at offset %v is not valid: %v (%v)", err.Off, err.Err, err.String)
+	return fmt.Sprintf("nbt: string at offset %v is not valid: %v (len=%v)", err.Off, err.Err, err.N)
 }
 
 const maximumNestingDepth = 512