Skip to content

Critical Fix. Buffer overflow caused random device reboot or hang. Malloc result checking in addReg and exceptionalResponce. Wrong value for every 8-th reg in read/writeCoils, readInputStats. #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions arduinoIDE/Modbus/Modbus.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
Modbus.cpp - Source for Modbus Base Library
Copyright (C) 2014 Andr� Sarmento Barbosa
Copyright (C) 2014 André Sarmento Barbosa
*/
#include "Modbus.h"

Expand All @@ -26,6 +26,7 @@ void Modbus::addReg(word address, word value) {
TRegister *newreg;

newreg = (TRegister *) malloc(sizeof(TRegister));
if (!newreg) return;
newreg->address = address;
newreg->value = value;
newreg->next = 0;
Expand Down Expand Up @@ -176,6 +177,10 @@ void Modbus::exceptionResponse(byte fcode, byte excode) {
free(_frame);
_len = 2;
_frame = (byte *) malloc(_len);
if (!_frame) {
_reply = MB_REPLY_OFF;
return;
}
_frame[0] = fcode + 0x80;
_frame[1] = excode;

Expand Down Expand Up @@ -324,17 +329,18 @@ void Modbus::readCoils(word startreg, word numregs) {
_frame[1] = _len - 2; //byte count (_len - function code and byte count)

byte bitn = 0;
word totregs = numregs;
word i;
word i = 0;
while (numregs--) {
i = (totregs - numregs) / 8;
if (this->Coil(startreg))
bitSet(_frame[2+i], bitn);
else
bitClear(_frame[2+i], bitn);
//increment the bit index
bitn++;
if (bitn == 8) bitn = 0;
if (bitn == 8) {
bitn = 0;
i++;
}
//increment the register
startreg++;
}
Expand All @@ -352,7 +358,7 @@ void Modbus::readInputStatus(word startreg, word numregs) {
//Check Address
//*** See comments on readCoils method.
if (!this->searchRegister(startreg + 10001)) {
this->exceptionResponse(MB_FC_READ_COILS, MB_EX_ILLEGAL_ADDRESS);
this->exceptionResponse(MB_FC_READ_INPUT_STAT, MB_EX_ILLEGAL_ADDRESS);
return;
}

Expand All @@ -375,17 +381,18 @@ void Modbus::readInputStatus(word startreg, word numregs) {
_frame[1] = _len - 2;

byte bitn = 0;
word totregs = numregs;
word i;
word i = 0;
while (numregs--) {
i = (totregs - numregs) / 8;
if (this->Ists(startreg))
bitSet(_frame[2+i], bitn);
else
bitClear(_frame[2+i], bitn);
//increment the bit index
bitn++;
if (bitn == 8) bitn = 0;
if (bitn == 8) {
bitn = 0;
i++;
}
//increment the register
startreg++;
}
Expand All @@ -403,7 +410,7 @@ void Modbus::readInputRegisters(word startreg, word numregs) {
//Check Address
//*** See comments on readCoils method.
if (!this->searchRegister(startreg + 30001)) {
this->exceptionResponse(MB_FC_READ_COILS, MB_EX_ILLEGAL_ADDRESS);
this->exceptionResponse(MB_FC_READ_INPUT_REGS, MB_EX_ILLEGAL_ADDRESS);
return;
}

Expand Down Expand Up @@ -494,14 +501,15 @@ void Modbus::writeMultipleCoils(byte* frame,word startreg, word numoutputs, byte
_frame[4] = numoutputs & 0x00FF;

byte bitn = 0;
word totoutputs = numoutputs;
word i;
word i = 0;
while (numoutputs--) {
i = (totoutputs - numoutputs) / 8;
this->Coil(startreg, bitRead(frame[6+i], bitn));
//increment the bit index
bitn++;
if (bitn == 8) bitn = 0;
if (bitn == 8) {
bitn = 0;
i++;
}
//increment the register
startreg++;
}
Expand Down
2 changes: 1 addition & 1 deletion arduinoIDE/ModbusIP_ESP8266/ModbusIP_ESP8266.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ void ModbusIP::task() {
_frame = (byte*) malloc(_len);

raw_len = raw_len - 7;
for (int i=0; i< raw_len; i++) _frame[i] = client.read(); //Get Modbus PDU
for (int i=0; i< _len; i++) _frame[i] = client.read(); //Get Modbus PDU

this->receivePDU(_frame);
client.flush();
Expand Down