Merge pull request #39 from Zondax/bugfix-decoding

Fix RLP & JSON decoding

Author: jleni

app/src/parser.c

@@ -29,6 +29,8 @@ void __assert_fail(const char * assertion, const char * file, unsigned int line,
 }
 #endif
 
+#define FLOW_PUBKEY_SIZE 150
+
 parser_error_t parser_parse(parser_context_t *ctx, const uint8_t *data, size_t dataLen) {
     CHECK_PARSER_ERR(parser_init(ctx, data, dataLen))
     return _read(ctx, &parser_tx_obj);
@@ -86,7 +88,7 @@ parser_error_t parser_printArgumentPublicKey(const parser_context_t *argumentCtx
     parsed_json_t parsedJson = {false};
     CHECK_PARSER_ERR(json_parse(&parsedJson, (char *) argumentCtx->buffer, argumentCtx->bufferLen));
 
-    char bufferUI[130];
+    char bufferUI[FLOW_PUBKEY_SIZE];
     CHECK_PARSER_ERR(json_extractPubKey(bufferUI, sizeof(bufferUI), &parsedJson, 0))
     pageString(outVal, outValLen, bufferUI, pageIdx, pageCount);
 
@@ -118,7 +120,7 @@ parser_error_t parser_printArgumentPublicKeys(const parser_context_t *argumentCt
     zemu_log_stack("PublicKeys");
 
     uint16_t arrayElementToken;
-    char bufferUI[130];
+    char bufferUI[FLOW_PUBKEY_SIZE];
     CHECK_PARSER_ERR(array_get_nth_element(&parsedJson, internalTokenElementIdx, argumentIndex, &arrayElementToken))
     CHECK_PARSER_ERR(json_extractPubKey(bufferUI, sizeof(bufferUI), &parsedJson, arrayElementToken))
     pageString(outVal, outValLen, bufferUI, pageIdx, pageCount);

app/src/parser_impl.c

@@ -335,7 +335,7 @@ parser_error_t _readGasLimit(parser_context_t *c, flow_gaslimit_t *v) {
 
     CHECK_PARSER_ERR(rlp_decode(c, &ctx_local, &kind, &bytesConsumed));
     CTX_CHECK_AND_ADVANCE(c, bytesConsumed)
-    CHECK_KIND(kind, kind_byte)
+    CHECK_KIND(kind, kind_string)
 
     CHECK_PARSER_ERR(rlp_readUInt64(&ctx_local, kind, v))
 
@@ -363,7 +363,7 @@ parser_error_t _readProposalKeyId(parser_context_t *c, flow_proposal_keyid_t *v)
 
     CHECK_PARSER_ERR(rlp_decode(c, &ctx_local, &kind, &bytesConsumed));
     CTX_CHECK_AND_ADVANCE(c, bytesConsumed)
-    CHECK_KIND(kind, kind_byte)
+    CHECK_KIND(kind, kind_string)
 
     CHECK_PARSER_ERR(rlp_readUInt64(&ctx_local, kind, v))
 
@@ -379,7 +379,7 @@ parser_error_t _readProposalKeySequenceNumber(parser_context_t *c, flow_proposal
 
     CHECK_PARSER_ERR(rlp_decode(c, &ctx_local, &kind, &bytesConsumed));
     CTX_CHECK_AND_ADVANCE(c, bytesConsumed)
-    CHECK_KIND(kind, kind_byte)
+    CHECK_KIND(kind, kind_string)
 
     CHECK_PARSER_ERR(rlp_readUInt64(&ctx_local, kind, v))
 

app/src/rlp.c

@@ -36,7 +36,7 @@ parser_error_t rlp_decode(
     uint8_t p = *outputPayload->buffer;
 
     if (p >= 0 && p <= 0x7F) {
-        *outputKind = kind_byte;
+        *outputKind = kind_string;
         outputPayload->bufferLen = 1;
         outputPayload->buffer += 0;
         *bytesConsumed = 1; // 1 byte to consume from the stream
@@ -99,7 +99,7 @@ parser_error_t rlp_decode(
 }
 
 parser_error_t rlp_readByte(const parser_context_t *ctx, rlp_kind_e kind, uint8_t *value) {
-    if (kind != kind_byte) {
+    if (kind != kind_string) {
         return parser_rlp_error_invalid_kind;
     }
 
@@ -118,28 +118,29 @@ parser_error_t rlp_readByte(const parser_context_t *ctx, rlp_kind_e kind, uint8_
 parser_error_t rlp_readUInt64(const parser_context_t *ctx,
                               rlp_kind_e kind,
                               uint64_t *value) {
-    if (kind == kind_string) {
-        *value = 0;
-
-        if (ctx->bufferLen != 8) {
-            return parser_rlp_error_invalid_value_len;
-        }
-
-        for (uint8_t i = 0; i < 8; i++) {
-            *value <<= 8u;
-            *value += *(ctx->buffer + ctx->offset + i);
-        }
-
-        *value <<= 8u;
-        return parser_ok;
+    if (kind != kind_string) {
+        return parser_rlp_error_invalid_kind;
     }
 
-    if (kind == kind_byte) {
+    // handle case when string is a single byte
+    if (ctx->bufferLen == 1) {
         uint8_t tmp;
         CHECK_PARSER_ERR(rlp_readByte(ctx, kind, &tmp))
         *value = tmp;
         return parser_ok;
     }
 
-    return parser_rlp_error_invalid_kind;
+    // max size of uint64_t is 8 bytes
+    if (ctx->bufferLen > 8) {
+        return parser_rlp_error_invalid_value_len;
+    }
+
+    *value = 0;
+
+    for (uint8_t i = 0; i < ctx->bufferLen; i++) {
+        *value <<= 8u;
+        *value += *(ctx->buffer + ctx->offset + i);
+    }
+
+    return parser_ok;
 }

app/src/rlp.h

@@ -25,7 +25,6 @@ extern "C" {
 
 typedef enum {
     kind_unknown = 0,
-    kind_byte,
     kind_string,
     kind_list,
 } rlp_kind_e;

tests/rlp.cpp

@@ -37,15 +37,18 @@ class RLPDecodeTest : public testing::TestWithParam<RLPValueTestCase> {
     void TearDown() override {}
 };
 
+// Test cases adapted from official Ethereum RLP tests: https://github.com/ethereum/tests/blob/develop/RLPTests/rlptest.json
 INSTANTIATE_TEST_SUITE_P(
         InstantiationName,
         RLPDecodeTest,
         Values(
-                RLPValueTestCase{"00", kind_byte, 1, 0, 1},
-                RLPValueTestCase{"01", kind_byte, 1, 0, 1},
-                RLPValueTestCase{"7F", kind_byte, 1, 0, 1},
+                RLPValueTestCase{"00", kind_string, 1, 0, 1}, // Byte string (00)
+                RLPValueTestCase{"01", kind_string, 1, 0, 1}, // Byte string (01)
+                RLPValueTestCase{"7F", kind_string, 1, 0, 1}, // Byte string (7F)
+
+                RLPValueTestCase{"80", kind_string, 0, 1, 1},       // Empty string ("")
+                RLPValueTestCase{"83646F67", kind_string, 3, 1, 4}, // Short string ("dog")
 
-                RLPValueTestCase{"80", kind_string, 0, 1, 1},
                 RLPValueTestCase{"B7"
                                  "0000000000000000000000000000000000000000000000000000000000000000"
                                  "0000000000000000000000000000000000000000000000000000000000000000"