From 51da2802aab30078f592db3abccc1304d33218ac Mon Sep 17 00:00:00 2001
From: Mark Spanbroek <mark@spanbroek.net>
Date: Wed, 1 Dec 2021 16:34:02 +0100
Subject: [PATCH] Decoding fails gracefully in several error cases

---
 contractabi/decoding.nim           | 31 +++++++++++++++++++++++-------
 tests/contractabi/testDecoding.nim | 27 +++++++++++++++++++-------
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/contractabi/decoding.nim b/contractabi/decoding.nim
index eac998a..6e876e0 100644
--- a/contractabi/decoding.nim
+++ b/contractabi/decoding.nim
@@ -45,23 +45,37 @@ func updateLast(decoder: var AbiDecoder, index: int) =
 func advance(decoder: var AbiDecoder, amount: int): ?!void =
   decoder.index += amount
   decoder.updateLast(decoder.index)
+  if decoder.index <= decoder.bytes.len:
+    success()
+  else:
+    failure "reading past end"
+
+func skipPadding(decoder: var AbiDecoder, amount: int): ?!void =
+  let index = decoder.index
+  ?decoder.advance(amount)
+  for i in index..<index+amount:
+    if decoder.bytes[i] != 0:
+      return failure "invalid padding found"
   success()
 
 func read(decoder: var AbiDecoder, amount: int, padding=padLeft): ?!seq[byte] =
   let padlen = (32 - amount mod 32) mod 32
   if padding == padLeft:
-    ?decoder.advance(padlen)
+    ?decoder.skipPadding(padlen)
   let index = decoder.index
   ?decoder.advance(amount)
   result = success decoder.bytes[index..<index+amount]
   if padding == padRight:
-    ?decoder.advance(padlen)
+    ?decoder.skipPadding(padlen)
 
 func decode(decoder: var AbiDecoder, T: type UInt): ?!T =
   success T.fromBytesBE(?decoder.read(sizeof(T)))
 
 func decode(decoder: var AbiDecoder, T: type bool): ?!T =
-  success (?decoder.read(uint8) != 0)
+  case ?decoder.read(uint8)
+    of 0: success false
+    of 1: success true
+    else: failure "invalid boolean value"
 
 func decode(decoder: var AbiDecoder, T: type enum): ?!T =
   success T(?decoder.read(uint64))
@@ -115,10 +129,13 @@ func decode[T: tuple](decoder: var AbiDecoder, _: typedesc[T]): ?!T =
 func read*(decoder: var AbiDecoder, T: type): ?!T =
   decoder.decode(T)
 
-func finish(decoder: var AbiDecoder) =
+func finish(decoder: var AbiDecoder): ?!void =
   doAssert decoder.stack.len == 1, "not all tuples were finished"
-  doAssert decoder.last == decoder.bytes.len, "unread trailing bytes found"
-  doAssert decoder.last mod 32 == 0, "encoding variant broken"
+  doAssert decoder.last mod 32 == 0, "encoding invariant broken"
+  if decoder.last != decoder.bytes.len:
+    failure "unread trailing bytes found"
+  else:
+    success()
 
 func decode[T](decoder: var AbiDecoder, _: type seq[T]): ?!seq[T] =
   var sequence: seq[T]
@@ -144,5 +161,5 @@ func decode(decoder: var AbiDecoder, T: type string): ?!T =
 func decode*(_: type AbiDecoder, bytes: seq[byte], T: type): ?!T =
   var decoder = AbiDecoder.init(bytes)
   var value = ?decoder.decode(T)
-  decoder.finish()
+  ?decoder.finish()
   success value
diff --git a/tests/contractabi/testDecoding.nim b/tests/contractabi/testDecoding.nim
index cfe8bd8..7384cd1 100644
--- a/tests/contractabi/testDecoding.nim
+++ b/tests/contractabi/testDecoding.nim
@@ -20,14 +20,31 @@ suite "ABI decoding":
     checkDecode(uint32)
     checkDecode(uint64)
 
-  # TODO: failure to decode when prefix not all zeroes
-  # TODO: failure when trailing bytes
+  test "fails to decode when reading past end":
+    var encoded = AbiEncoder.encode(uint8.example)
+    encoded.delete(encoded.len-1)
+    let decoded = AbiDecoder.decode(encoded, uint8)
+    check decoded.error.msg == "reading past end"
+
+  test "fails to decode when trailing bytes remain":
+    var encoded = AbiEncoder.encode(uint8.example)
+    encoded.add(byte.example)
+    let decoded = AbiDecoder.decode(encoded, uint8)
+    check decoded.error.msg == "unread trailing bytes found"
+
+  test "fails to decode when padding does not consist of zeroes":
+    var encoded = AbiEncoder.encode(uint8.example)
+    encoded[3] = 42'u8
+    let decoded = AbiDecoder.decode(encoded, uint8)
+    check decoded.error.msg == "invalid padding found"
 
   test "decodes booleans":
     checkDecode(false)
     checkDecode(true)
 
-  # TODO: failure to decode when value not 0 or 1
+  test "fails to decode boolean when value is not 0 or 1":
+    let encoded = AbiEncoder.encode(2'u8)
+    check AbiDecoder.decode(encoded, bool).error.msg == "invalid boolean value"
 
   test "decodes ranges":
     type SomeRange = range[0x0000'u16..0xAAAA'u16]
@@ -55,15 +72,11 @@ suite "ABI decoding":
     checkDecode(array[32, byte].example)
     checkDecode(array[33, byte].example)
 
-  # TODO: failure to decode when byte array of wrong length
-
   test "decodes byte sequences":
     checkDecode(@[1'u8, 2'u8, 3'u8])
     checkDecode(@(array[32, byte].example))
     checkDecode(@(array[33, byte].example))
 
-  # TODO: failure to decode when not enough bytes
-
   test "decodes tuples":
     let a = true
     let b = @[1'u8, 2'u8, 3'u8]