Skip to content

Commit 9613663

Browse files
committed
Fixed bug that caused the wrong message to be signed
process() altered the message buffer pending signing when called in sign() causing the wrong message to be sent to the signing backend. This is resolved by copying the original message before processing incoming messages to make sure the original message is kept. After it is successfully signed, it is copied back to its original location again. Also, the old 'ack' message buffer is renamed to tmpMsg to indicate its use as a temporary message buffer for various purposes.
1 parent 1cd84bf commit 9613663

File tree

2 files changed

+15
-11
lines changed

2 files changed

+15
-11
lines changed

libraries/MySensors/MySensor.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -349,12 +349,12 @@ boolean MySensor::process() {
349349
// Check if sender requests an ack back.
350350
if (mGetRequestAck(msg)) {
351351
// Copy message
352-
ack = msg;
353-
mSetRequestAck(ack,false); // Reply without ack flag (otherwise we would end up in an eternal loop)
354-
mSetAck(ack,true);
355-
ack.sender = nc.nodeId;
356-
ack.destination = msg.sender;
357-
sendRoute(ack);
352+
tmpMsg = msg;
353+
mSetRequestAck(tmpMsg,false); // Reply without ack flag (otherwise we would end up in an eternal loop)
354+
mSetAck(tmpMsg,true);
355+
tmpMsg.sender = nc.nodeId;
356+
tmpMsg.destination = msg.sender;
357+
sendRoute(tmpMsg);
358358
}
359359

360360
if (command == C_INTERNAL) {
@@ -591,18 +591,21 @@ int8_t MySensor::sleep(uint8_t interrupt1, uint8_t mode1, uint8_t interrupt2, ui
591591
}
592592

593593
bool MySensor::sign(MyMessage &message) {
594-
MyMessage msgNonce;
595-
if (!sendRoute(build(msgNonce, nc.nodeId, message.destination, message.sensor, C_INTERNAL, I_GET_NONCE, false).set(""))) {
594+
if (!sendRoute(build(tmpMsg, nc.nodeId, message.destination, message.sensor, C_INTERNAL, I_GET_NONCE, false).set(""))) {
596595
return false;
597596
} else {
598597
// We have to wait for the nonce to arrive before we can sign our original message
599598
// Other messages could come in-between. We trust process() takes care of them
600599
unsigned long enter = millis();
600+
msgSign = message; // Copy the message to sign since message buffer might be touched in process()
601601
while (millis() - enter < 5000) {
602602
if (process()) {
603-
if (getLastMessage().type == I_GET_NONCE_RESPONSE) {
603+
if (mGetCommand(getLastMessage()) == C_INTERNAL && getLastMessage().type == I_GET_NONCE_RESPONSE) {
604604
// Proceed with signing if nonce has been received
605-
if (signer.putNonce(getLastMessage()) && signer.signMsg(message)) return true;
605+
if (signer.putNonce(getLastMessage()) && signer.signMsg(msgSign)) {
606+
message = msgSign; // Write the signed message back
607+
return true;
608+
}
606609
break;
607610
}
608611
}

libraries/MySensors/MySensor.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ class MySensor
246246
uint16_t doSign[16]; // Bitfield indicating which sensors require signed communication
247247

248248
MyMessage msg; // Buffer for incoming messages.
249-
MyMessage ack; // Buffer for ack messages.
249+
MyMessage tmpMsg ; // Buffer for temporary messages (acks and nonces among others).
250+
MyMessage msgSign; // Buffer for message to sign.
250251

251252
MyRFDriverClass radio;
252253

0 commit comments

Comments
 (0)