From 5e5b06aa28fbe327fb45a20735fd26846218cc13 Mon Sep 17 00:00:00 2001 From: tekka007 Date: Thu, 31 Dec 2015 11:37:37 +0100 Subject: [PATCH 1/5] Fix payload null termination issue --- libraries/MySensors/core/MyTransport.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/libraries/MySensors/core/MyTransport.cpp b/libraries/MySensors/core/MyTransport.cpp index b01cc2d..a9edb5f 100644 --- a/libraries/MySensors/core/MyTransport.cpp +++ b/libraries/MySensors/core/MyTransport.cpp @@ -74,15 +74,18 @@ inline void transportProcess() { (void)signerCheckTimer(); // Manage signing timeout #endif - uint8_t len = transportReceive((uint8_t *)&_msg); - (void)len; //until somebody makes use of 'len' + uint8_t payloadLength = transportReceive((uint8_t *)&_msg); + (void)payloadLength; //until somebody makes use of it ledBlinkRx(1); uint8_t command = mGetCommand(_msg); + // prevent buffer overflow by limiting message length (5 bits=31 bytes max) to MAX_PAYLOAD (25 bytes) + uint8_t messageLength = min(mGetLength(_msg),MAX_PAYLOAD); uint8_t type = _msg.type; uint8_t sender = _msg.sender; uint8_t last = _msg.last; uint8_t destination = _msg.destination; + #ifdef MY_SIGNING_FEATURE // Before processing message, reject unsigned messages if signing is required and check signature (if it is signed and addressed to us) @@ -137,7 +140,9 @@ inline void transportProcess() { if (destination == _nc.nodeId) { // This message is addressed to this node mSetSigned(_msg,0); // Clear the sign-flag now as verification is completed - + // clear bytes after message end + memset((uint8_t*)&_msg.data[messageLength],0x00,MAX_PAYLOAD-messageLength); + #if defined(MY_REPEATER_FEATURE) if (_msg.last != _nc.parentNodeId) { // Message is from one of the child nodes. Add it to routing table. From 055bd2ff00f48390014fb8c390ee7a67e8bf85f9 Mon Sep 17 00:00:00 2001 From: tekka007 Date: Thu, 31 Dec 2015 12:02:40 +0100 Subject: [PATCH 2/5] Clear last byte of data buffer --- libraries/MySensors/core/MyTransport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/MySensors/core/MyTransport.cpp b/libraries/MySensors/core/MyTransport.cpp index a9edb5f..fd823f1 100644 --- a/libraries/MySensors/core/MyTransport.cpp +++ b/libraries/MySensors/core/MyTransport.cpp @@ -141,7 +141,7 @@ inline void transportProcess() { // This message is addressed to this node mSetSigned(_msg,0); // Clear the sign-flag now as verification is completed // clear bytes after message end - memset((uint8_t*)&_msg.data[messageLength],0x00,MAX_PAYLOAD-messageLength); + memset((uint8_t*)&_msg.data[messageLength],0x00,MAX_PAYLOAD-messageLength+1); #if defined(MY_REPEATER_FEATURE) if (_msg.last != _nc.parentNodeId) { From 823b97b6d985e083cb11bff9c4f799ab369de35e Mon Sep 17 00:00:00 2001 From: tekka007 Date: Thu, 31 Dec 2015 13:19:12 +0100 Subject: [PATCH 3/5] Last byte clearing is sufficient --- libraries/MySensors/core/MyTransport.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/MySensors/core/MyTransport.cpp b/libraries/MySensors/core/MyTransport.cpp index fd823f1..537d191 100644 --- a/libraries/MySensors/core/MyTransport.cpp +++ b/libraries/MySensors/core/MyTransport.cpp @@ -140,9 +140,8 @@ inline void transportProcess() { if (destination == _nc.nodeId) { // This message is addressed to this node mSetSigned(_msg,0); // Clear the sign-flag now as verification is completed - // clear bytes after message end - memset((uint8_t*)&_msg.data[messageLength],0x00,MAX_PAYLOAD-messageLength+1); - + // null terminate data + _msg.data[messageLength] = 0x00; #if defined(MY_REPEATER_FEATURE) if (_msg.last != _nc.parentNodeId) { // Message is from one of the child nodes. Add it to routing table. From 6d1c6099e8e0393c4636c35ef097affcd506c279 Mon Sep 17 00:00:00 2001 From: tekka007 Date: Thu, 31 Dec 2015 17:01:15 +0100 Subject: [PATCH 4/5] General limitation of msg length to MAX_PAYLOAD --- libraries/MySensors/core/MyTransport.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/MySensors/core/MyTransport.cpp b/libraries/MySensors/core/MyTransport.cpp index 537d191..57391b7 100644 --- a/libraries/MySensors/core/MyTransport.cpp +++ b/libraries/MySensors/core/MyTransport.cpp @@ -78,9 +78,8 @@ inline void transportProcess() { (void)payloadLength; //until somebody makes use of it ledBlinkRx(1); + uint8_t command = mGetCommand(_msg); - // prevent buffer overflow by limiting message length (5 bits=31 bytes max) to MAX_PAYLOAD (25 bytes) - uint8_t messageLength = min(mGetLength(_msg),MAX_PAYLOAD); uint8_t type = _msg.type; uint8_t sender = _msg.sender; uint8_t last = _msg.last; @@ -140,8 +139,11 @@ inline void transportProcess() { if (destination == _nc.nodeId) { // This message is addressed to this node mSetSigned(_msg,0); // Clear the sign-flag now as verification is completed + // prevent buffer overflow by limiting max. possible message length (5 bits=31 bytes max) to MAX_PAYLOAD (25 bytes) + mSetLength(_msg) = min(mGetLength(_msg),MAX_PAYLOAD); // null terminate data - _msg.data[messageLength] = 0x00; + _msg.data[mGetLength(_msg)] = 0x00; + #if defined(MY_REPEATER_FEATURE) if (_msg.last != _nc.parentNodeId) { // Message is from one of the child nodes. Add it to routing table. From cfed7a279d511d4a5a7700ef6f6d589cb5d34295 Mon Sep 17 00:00:00 2001 From: tekka007 Date: Thu, 31 Dec 2015 17:03:40 +0100 Subject: [PATCH 5/5] General limitation of msg length to MAX_PAYLOAD correction --- libraries/MySensors/core/MyTransport.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/MySensors/core/MyTransport.cpp b/libraries/MySensors/core/MyTransport.cpp index 57391b7..4c3e796 100644 --- a/libraries/MySensors/core/MyTransport.cpp +++ b/libraries/MySensors/core/MyTransport.cpp @@ -140,7 +140,7 @@ inline void transportProcess() { // This message is addressed to this node mSetSigned(_msg,0); // Clear the sign-flag now as verification is completed // prevent buffer overflow by limiting max. possible message length (5 bits=31 bytes max) to MAX_PAYLOAD (25 bytes) - mSetLength(_msg) = min(mGetLength(_msg),MAX_PAYLOAD); + mSetLength(_msg, min(mGetLength(_msg),MAX_PAYLOAD)); // null terminate data _msg.data[mGetLength(_msg)] = 0x00;