From 3b4dec2d12f571a845c56ec02ba3d03bcaea5569 Mon Sep 17 00:00:00 2001
From: moneromooo-monero <moneromooo-monero@users.noreply.github.com>
Date: Sat, 23 Jun 2018 15:26:22 +0100
Subject: [PATCH 1/3] device_ledger: fix potential buffer overflow from bad
 size calc

---
 src/device/device_ledger.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/device/device_ledger.cpp b/src/device/device_ledger.cpp
index aedaf8382..94cb3db89 100644
--- a/src/device/device_ledger.cpp
+++ b/src/device/device_ledger.cpp
@@ -194,7 +194,8 @@ namespace hw {
           this->buffer_send[3],
           this->buffer_send[4]
           );
-        buffer_to_str(strbuffer+strlen(strbuffer), sizeof(strbuffer), (char*)(this->buffer_send+5), this->length_send-5);
+        const size_t len = strlen(strbuffer);
+        buffer_to_str(strbuffer+len, sizeof(strbuffer)-len, (char*)(this->buffer_send+5), this->length_send-5);
         MDEBUG( "CMD  :" << strbuffer);
       }
     }
@@ -206,7 +207,8 @@ namespace hw {
           this->buffer_recv[this->length_recv-2],
           this->buffer_recv[this->length_recv-1]
           );
-        buffer_to_str(strbuffer+strlen(strbuffer), sizeof(strbuffer), (char*)(this->buffer_recv), this->length_recv-2);
+        const size_t len = strlen(strbuffer);
+        buffer_to_str(strbuffer+len, sizeof(strbuffer)-len, (char*)(this->buffer_recv), this->length_recv-2);
         MDEBUG( "RESP :" << strbuffer);
 
       }

From 41e9cab4e1c9d6df26db1cecd30fe6aafacc80a7 Mon Sep 17 00:00:00 2001
From: moneromooo-monero <moneromooo-monero@users.noreply.github.com>
Date: Sat, 23 Jun 2018 15:26:55 +0100
Subject: [PATCH 2/3] device: misc cleanup

use snprintf "just in case" where appropriate
consistently use unsigned for temp values
pass std::string by const ref rather than by value
add length check (which can't happen in practice) for memcpy
---
 src/device/device_ledger.cpp |  6 +++---
 src/device/log.cpp           | 12 +++++++-----
 src/device/log.hpp           |  8 ++++----
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/device/device_ledger.cpp b/src/device/device_ledger.cpp
index 94cb3db89..8735baeb6 100644
--- a/src/device/device_ledger.cpp
+++ b/src/device/device_ledger.cpp
@@ -187,7 +187,7 @@ namespace hw {
     void device_ledger::logCMD() {
       if (apdu_verbose) {
         char  strbuffer[1024];
-        sprintf(strbuffer, "%.02x %.02x %.02x %.02x %.02x ",
+        snprintf(strbuffer, sizeof(strbuffer), "%.02x %.02x %.02x %.02x %.02x ",
           this->buffer_send[0],
           this->buffer_send[1],
           this->buffer_send[2],
@@ -203,7 +203,7 @@ namespace hw {
     void device_ledger::logRESP() {
       if (apdu_verbose) {
         char  strbuffer[1024];
-        sprintf(strbuffer, "%.02x%.02x ",
+        snprintf(strbuffer, sizeof(strbuffer), "%.02x%.02x ",
           this->buffer_recv[this->length_recv-2],
           this->buffer_recv[this->length_recv-1]
           );
@@ -295,7 +295,7 @@ namespace hw {
      
     unsigned int device_ledger::exchange(unsigned int ok, unsigned int mask) {
       LONG rv;
-      int sw;
+      unsigned int sw;
 
       ASSERT_T0(this->length_send <= BUFFER_SEND_SIZE);
       logCMD();
diff --git a/src/device/log.cpp b/src/device/log.cpp
index cbbcfc953..1707524fb 100644
--- a/src/device/log.cpp
+++ b/src/device/log.cpp
@@ -45,13 +45,13 @@ namespace hw {
       }
     }
 
-    void log_hexbuffer(std::string msg,  const char* buff, size_t len) {
+    void log_hexbuffer(const std::string &msg,  const char* buff, size_t len) {
       char logstr[1025];
       buffer_to_str(logstr, sizeof(logstr),  buff, len);
       MDEBUG(msg<< ": " << logstr);
     }
 
-    void log_message(std::string msg,  std::string info ) {
+    void log_message(const std::string &msg, const std::string &info ) {
       MDEBUG(msg << ": " << info);
     }
 
@@ -122,16 +122,18 @@ namespace hw {
 
     rct::keyV decrypt(const rct::keyV &keys) {
         rct::keyV x ;
+        x.reserve(keys.size());
         for (unsigned int j = 0; j<keys.size(); j++) {
             x.push_back(decrypt(keys[j]));
         }
         return x;
     }
 
-    static void check(std::string msg, std::string info, const char *h, const char *d, int len, bool crypted) {
+    static void check(const std::string &msg, const std::string &info, const char *h, const char *d, size_t len, bool crypted) {
       char dd[32];
       char logstr[128];
 
+      CHECK_AND_ASSERT_THROW_MES(len <= sizeof(dd), "invalid len");
       memmove(dd,d,len);
       if (crypted) {
         CHECK_AND_ASSERT_THROW_MES(len<=32, "encrypted data greater than 32");
@@ -149,11 +151,11 @@ namespace hw {
       }
     }
 
-    void check32(std::string msg, std::string info, const char *h, const char *d, bool crypted) {
+    void check32(const std::string &msg, const std::string &info, const char *h, const char *d, bool crypted) {
       check(msg, info, h, d, 32, crypted);
     }
 
-    void check8(std::string msg, std::string info, const char *h, const char *d, bool crypted) {
+    void check8(const std::string &msg, const std::string &info, const char *h, const char *d, bool crypted) {
       check(msg, info, h, d, 8, crypted);
     }
     #endif
diff --git a/src/device/log.hpp b/src/device/log.hpp
index 1ab572c2c..1d1635dc1 100644
--- a/src/device/log.hpp
+++ b/src/device/log.hpp
@@ -44,8 +44,8 @@ namespace hw {
     namespace ledger {
 
         void buffer_to_str(char *to_buff,  size_t to_len, const char *buff, size_t len) ;
-        void log_hexbuffer(std::string msg,  const char* buff, size_t len);
-        void log_message(std::string msg,  std::string info );
+        void log_hexbuffer(const std::string &msg,  const char* buff, size_t len);
+        void log_message(const std::string &msg, const std::string &info );
         #ifdef DEBUG_HWDEVICE
         #define TRACK printf("file %s:%d\n",__FILE__, __LINE__)
         //#define TRACK MCDEBUG("ledger"," At file " << __FILE__ << ":" << __LINE__)
@@ -59,8 +59,8 @@ namespace hw {
         crypto::ec_scalar decrypt(const crypto::ec_scalar &res);
         rct::keyV decrypt(const rct::keyV &keys);
 
-        void check32(std::string msg, std::string info, const char *h, const char *d, bool crypted=false);
-        void check8(std::string msg, std::string info, const char *h, const char *d,  bool crypted=false);
+        void check32(const std::string &msg, const std::string &info, const char *h, const char *d, bool crypted=false);
+        void check8(const std::string &msg, const std::string &info, const char *h, const char *d,  bool crypted=false);
 
         void set_check_verbose(bool verbose);
         #endif

From 34de7bc267984340d0c6c0afcbb1069af2f81e6d Mon Sep 17 00:00:00 2001
From: moneromooo-monero <moneromooo-monero@users.noreply.github.com>
Date: Sat, 23 Jun 2018 15:43:31 +0100
Subject: [PATCH 3/3] device_ledger: fix buffer underflow on bad data from
 device

---
 src/device/device_ledger.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/device/device_ledger.cpp b/src/device/device_ledger.cpp
index 8735baeb6..bc7f37789 100644
--- a/src/device/device_ledger.cpp
+++ b/src/device/device_ledger.cpp
@@ -304,6 +304,7 @@ namespace hw {
                          SCARD_PCI_T0, this->buffer_send, this->length_send,
                          NULL,         this->buffer_recv, &this->length_recv);
       ASSERT_RV(rv);
+      ASSERT_T0(this->length_recv >= 2);
       ASSERT_T0(this->length_recv <= BUFFER_RECV_SIZE);
       logRESP();