unify and imporve extract errors

Author: jannotti

data/transactions/logic/box.go

@@ -74,14 +74,10 @@ func opBoxExtract(cx *EvalContext) error {
 		return err
 	}
 
-	end := start + length
-	if start > uint64(len(box)) || end > uint64(len(box)) {
-		return errors.New("extract range beyond box")
-	}
-
-	cx.stack[pprev].Bytes = []byte(box[start:end])
+	bytes, err := extractCarefully([]byte(box), start, length)
+	cx.stack[pprev].Bytes = bytes
 	cx.stack = cx.stack[:prev]
-	return nil
+	return err
 }
 
 func opBoxReplace(cx *EvalContext) error {

data/transactions/logic/box_test.go

@@ -69,7 +69,7 @@ func TestBoxReadWrite(t *testing.T) {
                 int 1`, ep)
 
 	logic.TestApp(t, `byte "self"; int 1; int 4; box_extract;
-                byte 0x00000000; ==`, ep, "extract range")
+                byte 0x00000000; ==`, ep, "extraction end 5")
 
 	// Replace some bytes until past the end, confirm when it fails.
 	logic.TestApp(t, `byte "self"; int 1; byte 0x3031; box_replace;

data/transactions/logic/eval.go

@@ -3494,39 +3494,43 @@ func opSetByte(cx *EvalContext) error {
 	return nil
 }
 
-func opExtractImpl(x []byte, start, length int) ([]byte, error) {
+func extractCarefully(x []byte, start, length uint64) ([]byte, error) {
+	if start > uint64(len(x)) {
+		return nil, fmt.Errorf("extraction start %d beyond length: %d", start, len(x))
+	}
 	end := start + length
-	if start > len(x) || end > len(x) {
-		return nil, errors.New("extract range beyond length of string")
+	if end < start {
+		return nil, fmt.Errorf("extraction end exceeds uint64")
+	}
+	if end > uint64(len(x)) {
+		return nil, fmt.Errorf("extraction end %d beyond length: %d", end, len(x))
 	}
 	return x[start:end], nil
 }
 
 func opExtract(cx *EvalContext) error {
 	last := len(cx.stack) - 1
-	startIdx := cx.program[cx.pc+1]
-	lengthIdx := cx.program[cx.pc+2]
+	start := uint64(cx.program[cx.pc+1])
+	length := uint64(cx.program[cx.pc+2])
 	// Shortcut: if length is 0, take bytes from start index to the end
-	length := int(lengthIdx)
 	if length == 0 {
-		length = len(cx.stack[last].Bytes) - int(startIdx)
+		// If length has wrapped, it's because start > len(), so extractCarefully will report
+		length = uint64(len(cx.stack[last].Bytes) - int(start))
 	}
-	bytes, err := opExtractImpl(cx.stack[last].Bytes, int(startIdx), length)
+	bytes, err := extractCarefully(cx.stack[last].Bytes, start, length)
 	cx.stack[last].Bytes = bytes
 	return err
 }
 
 func opExtract3(cx *EvalContext) error {
 	last := len(cx.stack) - 1 // length
 	prev := last - 1          // start
-	byteArrayIdx := prev - 1  // bytes
-	startIdx := cx.stack[prev].Uint
-	lengthIdx := cx.stack[last].Uint
-	if startIdx > math.MaxInt32 || lengthIdx > math.MaxInt32 {
-		return errors.New("extract range beyond length of string")
-	}
-	bytes, err := opExtractImpl(cx.stack[byteArrayIdx].Bytes, int(startIdx), int(lengthIdx))
-	cx.stack[byteArrayIdx].Bytes = bytes
+	pprev := prev - 1         // bytes
+
+	start := cx.stack[prev].Uint
+	length := cx.stack[last].Uint
+	bytes, err := extractCarefully(cx.stack[pprev].Bytes, start, length)
+	cx.stack[pprev].Bytes = bytes
 	cx.stack = cx.stack[:prev]
 	return err
 }
@@ -3542,11 +3546,11 @@ func convertBytesToInt(x []byte) uint64 {
 	return out
 }
 
-func opExtractNBytes(cx *EvalContext, n int) error {
+func opExtractNBytes(cx *EvalContext, n uint64) error {
 	last := len(cx.stack) - 1 // start
 	prev := last - 1          // bytes
-	startIdx := cx.stack[last].Uint
-	bytes, err := opExtractImpl(cx.stack[prev].Bytes, int(startIdx), n) // extract n bytes
+	start := cx.stack[last].Uint
+	bytes, err := extractCarefully(cx.stack[prev].Bytes, start, n) // extract n bytes
 	if err != nil {
 		return err
 	}

data/transactions/logic/eval_test.go

@@ -129,6 +129,7 @@ func defaultEvalParamsWithVersion(version uint64, txns ...transactions.SignedTxn
 		txns = []transactions.SignedTxn{{Txn: transactions.Transaction{Type: protocol.ApplicationCallTx}}}
 	}
 	ep := NewEvalParams(transactions.WrapSignedTxnsWithAD(txns), makeTestProtoV(version), &transactions.SpecialAddresses{})
+	ep.Trace = &strings.Builder{}
 	if empty {
 		// We made an app type in order to get a full ep, but that sets MinTealVersion=2
 		ep.TxnGroup[0].Txn.Type = "" // set it back
@@ -2299,41 +2300,41 @@ func TestExtractFlop(t *testing.T) {
 	err := testPanics(t, `byte 0xf000000000000000
 	extract 1 8
 	len`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction end 9")
 
 	err = testPanics(t, `byte 0xf000000000000000
 	extract 9 0
 	len`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction start 9")
 
 	err = testPanics(t, `byte 0xf000000000000000
 	int 4
 	int 0xFFFFFFFFFFFFFFFE
 	extract3
 	len`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction end exceeds uint64")
 
 	err = testPanics(t, `byte 0xf000000000000000
 	int 100
 	int 2
 	extract3
 	len`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction start 100")
 
 	err = testPanics(t, `byte 0xf000000000000000
 	int 55
 	extract_uint16`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction start 55")
 
 	err = testPanics(t, `byte 0xf000000000000000
 	int 9
 	extract_uint32`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction start 9")
 
 	err = testPanics(t, `byte 0xf000000000000000
 	int 1
 	extract_uint64`, 5)
-	require.Contains(t, err.Error(), "extract range beyond length of string")
+	require.Contains(t, err.Error(), "extraction end 9")
 }
 
 func TestLoadStore(t *testing.T) {