Merge pull request #79 from blooo-io/fix/LDG-696--nano-app-audit-fix-…

…part-2

Fix/ldg 696  nano app audit fix part 2

Makefile

@@ -30,7 +30,7 @@ APPNAME = "Concordium"
 # Application version
 APPVERSION_M = 5
 APPVERSION_N = 1
-APPVERSION_P = 1
+APPVERSION_P = 2
 APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"
 
 DEFINES += APPVERSION=\"$(APPVERSION)\"

src/common/sign.c

@@ -28,8 +28,13 @@ void buildAndSignTransactionHash() {
 }
 
 void readCborInitial(uint8_t *cdata, uint8_t dataLength) {
+    uint8_t remainingDataLength = dataLength;
+    if (remainingDataLength < 1) {
+        THROW(ERROR_BUFFER_OVERFLOW);
+    }
     uint8_t header = cdata[0];
     cdata += 1;
+    remainingDataLength -= 1;
     ctx->cborLength -= 1;
     // the first byte of an cbor encoding contains the type (3 high bits) and the shortCount (5
     // lower bits);
@@ -48,25 +53,25 @@ void readCborInitial(uint8_t *cdata, uint8_t dataLength) {
         // shortCount is the length, no extra bytes are used.
         length = shortCount;
     } else if (shortCount == CBOR_ONE_BYTE_LENGTH) {
-        if (dataLength < 1) {
+        if (remainingDataLength < 1) {
             THROW(ERROR_BUFFER_OVERFLOW);
         }
         length = cdata[0];
         sizeLength = 1;
     } else if (shortCount == CBOR_TWO_BYTE_LENGTH) {
-        if (dataLength < 2) {
+        if (remainingDataLength < 2) {
             THROW(ERROR_BUFFER_OVERFLOW);
         }
         length = U2BE(cdata, 0);
         sizeLength = 2;
     } else if (shortCount == CBOR_FOUR_BYTE_LENGTH) {
-        if (dataLength < 4) {
+        if (remainingDataLength < 4) {
             THROW(ERROR_BUFFER_OVERFLOW);
         }
         length = U4BE(cdata, 0);
         sizeLength = 4;
     } else if (shortCount == CBOR_EIGHT_BYTE_LENGTH) {
-        if (dataLength < 8) {
+        if (remainingDataLength < 8) {
             THROW(ERROR_BUFFER_OVERFLOW);
         }
         length = U8BE(cdata, 0);
@@ -77,7 +82,6 @@ void readCborInitial(uint8_t *cdata, uint8_t dataLength) {
         THROW(ERROR_INVALID_PARAM);
     }
     cdata += sizeLength;
-
     ctx->cborLength -= sizeLength;
     switch (ctx->majorType) {
         case 0:

src/exportPrivateKey.c

@@ -65,6 +65,9 @@ void exportPrivateKeyBls(void) {
             ctx->path[lastSubPathIndex] = lastSubPath | HARDENED_OFFSET;
             getBlsPrivateKey(ctx->path, lastSubPathIndex + 1, privateKey, sizeof(privateKey));
             uint8_t tx = 0;
+            if (sizeof(privateKey) > sizeof(G_io_apdu_buffer)) {
+                THROW(ERROR_BUFFER_OVERFLOW);
+            }
             memmove(G_io_apdu_buffer, privateKey, sizeof(privateKey));
             tx += sizeof(privateKey);
 
@@ -76,6 +79,9 @@ void exportPrivateKeyBls(void) {
                 }
                 ctx->path[lastSubPathIndex] = lastSubPath | HARDENED_OFFSET;
                 getBlsPrivateKey(ctx->path, lastSubPathIndex + 1, privateKey, sizeof(privateKey));
+                if (sizeof(privateKey) + tx > sizeof(G_io_apdu_buffer)) {
+                    THROW(ERROR_BUFFER_OVERFLOW);
+                }
                 memmove(G_io_apdu_buffer + tx, privateKey, sizeof(privateKey));
                 tx += sizeof(privateKey);
             }

src/signConfigureBaker.c

@@ -268,7 +268,9 @@ void handleSignConfigureBaker(uint8_t *cdata,
         if (!ctx_conf_baker->hasMetadataUrl) {
             THROW(ERROR_INVALID_TRANSACTION);
         }
-
+        if (dataLength < 2) {
+            THROW(ERROR_BUFFER_OVERFLOW);
+        }
         ctx_conf_baker->url.urlLength = U2BE(cdata, 0);
         if (ctx_conf_baker->url.urlLength > 2048) {
             THROW(ERROR_INVALID_TRANSACTION);
@@ -322,6 +324,9 @@ void handleSignConfigureBaker(uint8_t *cdata,
         handleCommissionRates(cdata, dataLength);
         *flags |= IO_ASYNCH_REPLY;
     } else if (P1_SUSPENDED == p1 && ctx_conf_baker->state == CONFIGURE_BAKER_SUSPENDED) {
+        if (dataLength < 1) {
+            THROW(ERROR_INVALID_TRANSACTION);
+        }
         uint8_t suspended = cdata[0];
         updateHash((cx_hash_t *)&tx_state->hash, cdata, 1);
         dataLength -= 1;

src/signCredentialDeployment.c

@@ -14,10 +14,10 @@ void processNextVerificationKey(void) {
 
 void parseVerificationKey(uint8_t *buffer, uint8_t dataLength) {
     // Hash key index
-    updateHash((cx_hash_t *)&tx_state->hash, buffer, 1);
     if (dataLength < 1) {
         THROW(ERROR_BUFFER_OVERFLOW);  // Ensure safe access
     }
+    updateHash((cx_hash_t *)&tx_state->hash, buffer, 1);
     dataLength -= 1;
     buffer += 1;
 
@@ -267,6 +267,9 @@ void handleSignCredentialDeployment(uint8_t *dataBuffer,
         int offset = numberToText(ctx->anonymityRevocationThreshold,
                                   sizeof(ctx->anonymityRevocationThreshold),
                                   dataBuffer[0]);
+        if ((size_t)(offset + 8) > sizeof(ctx->anonymityRevocationThreshold)) {
+            THROW(ERROR_BUFFER_OVERFLOW);
+        }
         memmove(ctx->anonymityRevocationThreshold + offset, " out of ", 8);
         offset += 8;
         updateHash((cx_hash_t *)&tx_state->hash, dataBuffer, 1);