Merge pull request #4043

34de7bc2 device_ledger: fix buffer underflow on bad data from device (moneromooo-monero)
41e9cab4 device: misc cleanup (moneromooo-monero)
3b4dec2d device_ledger: fix potential buffer overflow from bad size calc (moneromooo-monero)
This commit is contained in:
Riccardo Spagni 2018-07-03 15:21:56 +02:00
commit d1f102626c
No known key found for this signature in database
GPG key ID: 55432DF31CCD4FCD
3 changed files with 19 additions and 14 deletions

View file

@ -187,14 +187,15 @@ namespace hw {
void device_ledger::logCMD() { void device_ledger::logCMD() {
if (apdu_verbose) { if (apdu_verbose) {
char strbuffer[1024]; 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[0],
this->buffer_send[1], this->buffer_send[1],
this->buffer_send[2], this->buffer_send[2],
this->buffer_send[3], this->buffer_send[3],
this->buffer_send[4] 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); MDEBUG( "CMD :" << strbuffer);
} }
} }
@ -202,11 +203,12 @@ namespace hw {
void device_ledger::logRESP() { void device_ledger::logRESP() {
if (apdu_verbose) { if (apdu_verbose) {
char strbuffer[1024]; 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-2],
this->buffer_recv[this->length_recv-1] 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); MDEBUG( "RESP :" << strbuffer);
} }
@ -293,7 +295,7 @@ namespace hw {
unsigned int device_ledger::exchange(unsigned int ok, unsigned int mask) { unsigned int device_ledger::exchange(unsigned int ok, unsigned int mask) {
LONG rv; LONG rv;
int sw; unsigned int sw;
ASSERT_T0(this->length_send <= BUFFER_SEND_SIZE); ASSERT_T0(this->length_send <= BUFFER_SEND_SIZE);
logCMD(); logCMD();
@ -302,6 +304,7 @@ namespace hw {
SCARD_PCI_T0, this->buffer_send, this->length_send, SCARD_PCI_T0, this->buffer_send, this->length_send,
NULL, this->buffer_recv, &this->length_recv); NULL, this->buffer_recv, &this->length_recv);
ASSERT_RV(rv); ASSERT_RV(rv);
ASSERT_T0(this->length_recv >= 2);
ASSERT_T0(this->length_recv <= BUFFER_RECV_SIZE); ASSERT_T0(this->length_recv <= BUFFER_RECV_SIZE);
logRESP(); logRESP();

View file

@ -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]; char logstr[1025];
buffer_to_str(logstr, sizeof(logstr), buff, len); buffer_to_str(logstr, sizeof(logstr), buff, len);
MDEBUG(msg<< ": " << logstr); 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); MDEBUG(msg << ": " << info);
} }
@ -122,16 +122,18 @@ namespace hw {
rct::keyV decrypt(const rct::keyV &keys) { rct::keyV decrypt(const rct::keyV &keys) {
rct::keyV x ; rct::keyV x ;
x.reserve(keys.size());
for (unsigned int j = 0; j<keys.size(); j++) { for (unsigned int j = 0; j<keys.size(); j++) {
x.push_back(decrypt(keys[j])); x.push_back(decrypt(keys[j]));
} }
return x; 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 dd[32];
char logstr[128]; char logstr[128];
CHECK_AND_ASSERT_THROW_MES(len <= sizeof(dd), "invalid len");
memmove(dd,d,len); memmove(dd,d,len);
if (crypted) { if (crypted) {
CHECK_AND_ASSERT_THROW_MES(len<=32, "encrypted data greater than 32"); 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); 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); check(msg, info, h, d, 8, crypted);
} }
#endif #endif

View file

@ -44,8 +44,8 @@ namespace hw {
namespace ledger { namespace ledger {
void buffer_to_str(char *to_buff, size_t to_len, const char *buff, size_t len) ; 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_hexbuffer(const std::string &msg, const char* buff, size_t len);
void log_message(std::string msg, std::string info ); void log_message(const std::string &msg, const std::string &info );
#ifdef DEBUG_HWDEVICE #ifdef DEBUG_HWDEVICE
#define TRACK printf("file %s:%d\n",__FILE__, __LINE__) #define TRACK printf("file %s:%d\n",__FILE__, __LINE__)
//#define TRACK MCDEBUG("ledger"," At file " << __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); crypto::ec_scalar decrypt(const crypto::ec_scalar &res);
rct::keyV decrypt(const rct::keyV &keys); 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 check32(const std::string &msg, const 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 check8(const std::string &msg, const std::string &info, const char *h, const char *d, bool crypted=false);
void set_check_verbose(bool verbose); void set_check_verbose(bool verbose);
#endif #endif