From 20ea8e55af0173dc21d8205a929c7ead47163182 Mon Sep 17 00:00:00 2001 From: Mascaro Lucas Date: Thu, 13 Dec 2018 14:23:23 +0100 Subject: [PATCH] Fix buffer overflow errors +remove dynamic memory allocation +memset the rest of the data structure after read --- node/main/main.ino | 23 ++++++++++++----------- packet.cpp | 28 ++++++++++++---------------- packet.h | 12 +++--------- protocol.h | 14 ++++++-------- xbee_wrapper.cpp | 7 +++---- 5 files changed, 36 insertions(+), 48 deletions(-) diff --git a/node/main/main.ino b/node/main/main.ino index b195e8f..a5698f7 100644 --- a/node/main/main.ino +++ b/node/main/main.ino @@ -13,8 +13,6 @@ XBeeWrapper xbee = XBeeWrapper(); // ACTUAL DATA unsigned long time; Node myself = Node(); -Packet recv = Packet(); -Packet send = Packet(); void setup() { Serial.begin(38400); @@ -36,6 +34,7 @@ void loop() { } /* [2] Listen for incoming data */ + Packet recv; if( xbee.receive(recv) != XBWRECV_OK ) return; @@ -43,11 +42,11 @@ void loop() { // 1. manage discover request if( opcode == OPCODE_DISCOVER ) - return manage_discover(); + return manage_discover(recv); // 2. manage message data else if( opcode == OPCODE_MESSAGE ) - return manage_message(); + return manage_message(recv); } @@ -55,6 +54,7 @@ void loop() { void send_data(){ Serial.println(" -> message"); // 1. prepare message + Packet send; send.setOpcode(OPCODE_MESSAGE); send.setSender(SENDERID); send.setDist(myself.getDist()); @@ -66,15 +66,15 @@ void send_data(){ // 2. broadcast xbee.broadcast(send); - // 2. debug - screen.clear(); - screen.printfn(0, "msg[%3d/%3d] %3d", send.getDist(), send.getTTL(), send.getSize()); - screen.printfn_overflow(1, "%s", (char*) send.getData()); - delay(500); + // 2. debug + screen.clear(); + screen.printfn(0, "msg[%3d/%3d] %3d", send.getDist(), send.getTTL(), send.getSize()); + screen.printfn_overflow(1, "%s", (char*) send.getData()); + delay(500); } -void manage_discover(){ +void manage_discover(Packet recv){ Serial.print(" <- discover[ "); Serial.print(recv.getDist()); Serial.print(" / "); @@ -91,6 +91,7 @@ void manage_discover(){ } // propagate wave + Packet send; send.setOpcode(OPCODE_DISCOVER); send.setWave(myself.getWave()); send.setDist(myself.getDist()); @@ -99,7 +100,7 @@ void manage_discover(){ xbee.broadcast(send); } -void manage_message(){ +void manage_message(Packet recv){ if( recv.getTTL() <= 0 ) return; diff --git a/packet.cpp b/packet.cpp index fc22a52..1701359 100644 --- a/packet.cpp +++ b/packet.cpp @@ -1,18 +1,13 @@ #include "packet.h" -Packet::Packet(){ - msg.data = malloc(1 * sizeof(char)); -} -Packet::~Packet(){ - free(msg.data); -} + /* PUBLIC ----------------------------------------*/ // builds a packet from raw data and returns the error code uint8_t Packet::read(uint8_t* buf, const size_t size){ // 1. fail on invalid size if( size < 1 ) return PKTREAD_EMPTY; - if( size > PROTO_SIZE ) return PKTREAD_OVERFLOW; + if( size > PROTO_SIZE ) return PKTREAD_OVERFLOW; // 2. extract packet type opcode = buf[0]; @@ -51,13 +46,12 @@ uint8_t Packet::getSize() { return msg.size; } uint8_t* Packet::getData(){ return msg.data; } void Packet::setData(uint8_t *buffer) { - if( strlen(buffer) >= 255 ) + if( strlen(buffer) >= MESSAGE_MAX_PAYLOAD ) return; msg.size = strlen(buffer); - resizeMessage(); - strcpy(msg.data, buffer); + strncpy(msg.data, buffer, MESSAGE_MAX_PAYLOAD); } @@ -88,7 +82,7 @@ size_t Packet::write_discover(uint8_t *buf){ uint8_t Packet::read_message(uint8_t *buf, const size_t size){ // 1. fail on invalid size - if( size < MESSAGE_MIN_SIZE || size > MESSAGE_MAX_SIZE ) + if( size < PROTO_MIN_SIZE || size > PROTO_SIZE ) return PKTREAD_INVALID_MESSAGE_FORMAT; // 2. fill values @@ -102,9 +96,12 @@ uint8_t Packet::read_message(uint8_t *buf, const size_t size){ if( size - 5 != msg.size ) return PKTREAD_INVALID_MESSAGE_FORMAT; - // 4. extract message - resizeMessage(); + // 4. memset data + //memset(msg.data,0,MESSAGE_MAX_PAYLOAD); + + // 5. extract message strncpy(msg.data, buf+5, msg.size); + memset(msg.data+msg.size,0,MESSAGE_MAX_PAYLOAD-msg.size); return PKTREAD_OK; }; @@ -116,8 +113,7 @@ size_t Packet::write_message(uint8_t *buf){ buf[3] = msg.ttl; buf[4] = msg.size; - buf = realloc(buf, (5+msg.size+1)* sizeof(uint8_t)); - strncpy(buf+5, msg.data, msg.size); + strncpy(buf+5, msg.data, MESSAGE_MAX_PAYLOAD); return 5 + msg.size; -}; \ No newline at end of file +}; diff --git a/packet.h b/packet.h index a71bcc8..c8be864 100644 --- a/packet.h +++ b/packet.h @@ -26,14 +26,13 @@ size_t write_message(uint8_t* buf); public: - - Packet(); - ~Packet(); + // builds a packet from raw data and returns the status code uint8_t read(uint8_t* buf, const size_t size); // writes the binary representation of the packet returns the size size_t write(uint8_t* buf); + // GETTERS / SETTERS uint8_t getOpcode(); @@ -56,12 +55,7 @@ uint8_t* getData(); void setData(uint8_t *buffer); - protected: - resizeMessage(){ - msg.data = realloc(msg.data, (msg.size+1) * sizeof(uint8_t)); - memset(msg.data, 0, msg.size+1); - } }; -#endif \ No newline at end of file +#endif diff --git a/protocol.h b/protocol.h index 653f8c5..72e54a7 100644 --- a/protocol.h +++ b/protocol.h @@ -18,21 +18,19 @@ uint8_t dist; // current node's distance }; - - #define MESSAGE_MIN_SIZE sizeof(uint8_t)*5 - #define MESSAGE_MAX_SIZE (5 + 255) * sizeof(uint8_t) + #define MESSAGE_MAX_PAYLOAD 255 + #define PROTO_MIN_SIZE 5 * sizeof(uint8_t) + #define PROTO_SIZE (5 + MESSAGE_MAX_PAYLOAD) * sizeof(uint8_t) + struct message { uint8_t opcode; // opcode = 1 uint8_t sender; // sender id uint8_t dist; // distance of the last sender uint8_t ttl; // time to live default = 10 uint8_t size; // size of message in bytes - uint8_t *data; // actual message + uint8_t data[MESSAGE_MAX_PAYLOAD]; // actual message }; - #define PROTO_SIZE MESSAGE_MAX_SIZE - - class Node{ private: uint8_t wave = 0; @@ -58,4 +56,4 @@ } }; -#endif \ No newline at end of file +#endif diff --git a/xbee_wrapper.cpp b/xbee_wrapper.cpp index 188da3d..fa76377 100644 --- a/xbee_wrapper.cpp +++ b/xbee_wrapper.cpp @@ -49,14 +49,13 @@ uint8_t XBeeWrapper::broadcast(Packet& pkt){ XBeeAddress64 bcast = XBeeAddress64(0x00000000, 0x0000FFFF); // build payload from packet - uint8_t* payload = malloc(6 * sizeof(uint8_t)); + uint8_t payload[PROTO_SIZE]; + memset(payload,0,PROTO_SIZE); size_t payload_size = pkt.write(payload); // send Tx64Request tx = Tx64Request(bcast, payload, payload_size); xbee.send(tx); - free(payload); - return XBWSEND_OK; -}; \ No newline at end of file +};