Skip to content

Commit 209ae3d

Browse files
author
Craig Versek
committed
FIXED MAJOR FENCEPOST BUG, off by one on checking for _maxCommands being exceeded, which allowed out of bounds access to memory
1 parent 506232d commit 209ae3d

File tree

3 files changed

+78
-63
lines changed

3 files changed

+78
-63
lines changed

‎PacketCommand.cpp

+69-60
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ PacketShared::STATUS PacketCommand::addCommand(const byte* type_id,
9494
size_t type_id_len = strlen((char*) type_id);
9595
struct CommandInfo new_command;
9696
#ifdef PACKETCOMMAND_DEBUG
97-
Serial.print(F("Adding command #("));
97+
Serial.println(F("# in PacketCommand::addCommand"));
98+
Serial.print(F("#\tAdding command #("));
9899
Serial.print(_commandCount);
99100
Serial.print(F("): "));
100101
Serial.println(name);
@@ -103,16 +104,16 @@ PacketShared::STATUS PacketCommand::addCommand(const byte* type_id,
103104
Serial.println("'");
104105
Serial.println(type_id_len);
105106
#endif
106-
if (_commandCount >= _maxCommands){
107+
if (_commandCount >= _maxCommands - 1){
107108
#ifdef PACKETCOMMAND_DEBUG
108-
Serial.print(F("Error: exceeded maxCommands="));
109+
Serial.print(F("### Error: exceeded maxCommands="));
109110
Serial.println(_maxCommands);
110111
#endif
111112
return PacketShared::ERROR_EXCEDED_MAX_COMMANDS;
112113
}
113114
if (type_id_len > MAX_TYPE_ID_LEN){
114115
#ifdef PACKETCOMMAND_DEBUG
115-
Serial.print(F("Error: 'type_id' cannot exceed MAX_TYPE_ID_LEN="));
116+
Serial.print(F("### Error: 'type_id' cannot exceed MAX_TYPE_ID_LEN="));
116117
Serial.println(MAX_TYPE_ID_LEN);
117118
#endif
118119
return PacketShared::ERROR_INVALID_TYPE_ID;
@@ -123,43 +124,43 @@ PacketShared::STATUS PacketCommand::addCommand(const byte* type_id,
123124
//test if the type ID rules are followed
124125
cur_byte = type_id[i];
125126
#ifdef PACKETCOMMAND_DEBUG
126-
Serial.print(F("i="));
127+
Serial.print(F("#\ti="));
127128
Serial.println(i);
128-
Serial.print(F("checking type ID byte: "));
129+
Serial.print(F("#\tchecking type ID byte: "));
129130
Serial.println(cur_byte, HEX);
130131
#endif
131132
switch(cur_byte){
132133
case 0xFF:
133134
if (i < (type_id_len - 1)){
134135
//continue extended type ID
135136
#ifdef PACKETCOMMAND_DEBUG
136-
Serial.println(F("\tcontinue extended type ID"));
137+
Serial.println(F("#\tcontinue extended type ID"));
137138
#endif
138139
new_command.type_id[i] = 0xFF;
139140
}
140141
else{//cannot end type_id with 0xFF
141142
#ifdef PACKETCOMMAND_DEBUG
142-
Serial.println(F("Error: 'type_id' cannot end with 0xFF"));
143+
Serial.println(F("### Error: 'type_id' cannot end with 0xFF"));
143144
#endif
144145
return PacketShared::ERROR_INVALID_TYPE_ID;
145146
}
146147
break;
147148
case 0x00:
148149
#ifdef PACKETCOMMAND_DEBUG
149-
Serial.println(F("Error: 'type_id' cannot contain null (0x00) bytes"));
150+
Serial.println(F("### Error: 'type_id' cannot contain null (0x00) bytes"));
150151
#endif
151152
return PacketShared::ERROR_INVALID_TYPE_ID;
152153
break;
153154
default: //any other byte value
154155
if(i == (type_id_len - 1)){//valid type ID completed
155156
#ifdef PACKETCOMMAND_DEBUG
156-
Serial.println(F("valid type ID completed"));
157+
Serial.println(F("#\tvalid type ID completed"));
157158
#endif
158159
new_command.type_id[i] = cur_byte;
159160
}
160161
else{
161162
#ifdef PACKETCOMMAND_DEBUG
162-
Serial.println(F("Error: 'type_id' cannot have a prefix != [0xFF]*"));
163+
Serial.println(F("### Error: 'type_id' cannot have a prefix != [0xFF]*"));
163164
#endif
164165
return PacketShared::ERROR_INVALID_TYPE_ID;
165166
}
@@ -172,7 +173,7 @@ PacketShared::STATUS PacketCommand::addCommand(const byte* type_id,
172173
}
173174
}
174175
#ifdef PACKETCOMMAND_DEBUG
175-
Serial.print(F("type_id="));
176+
Serial.print(F("#\ttype_id="));
176177
for(size_t i=0; i < MAX_TYPE_ID_LEN; i++){
177178
if( new_command.type_id[i] != 0x00 ){
178179
Serial.print(new_command.type_id[i], HEX);
@@ -277,26 +278,26 @@ PacketShared::STATUS PacketCommand::registerReplyRecvCallback(bool (*function)(P
277278
*/
278279
PacketShared::STATUS PacketCommand::processInput(){
279280
#ifdef PACKETCOMMAND_DEBUG
280-
Serial.println(F("In PacketCommand::processInput"));
281-
Serial.print(F("\t_input_index="));Serial.println(_input_index);
282-
Serial.print(F("\t_input_len="));Serial.println(_input_len);
281+
Serial.println(F("# In PacketCommand::processInput"));
282+
Serial.print(F("#\t_input_index="));Serial.println(_input_index);
283+
Serial.print(F("#\t_input_len="));Serial.println(_input_len);
283284
#endif
284285
PacketShared::STATUS pcs = matchCommand();
285286
#ifdef PACKETCOMMAND_DEBUG
286-
Serial.println(F("(processInput)-after calling matchCommand()"));
287-
Serial.print(F("\t_input_index="));Serial.println(_input_index);
288-
Serial.print(F("\t_input_len="));Serial.println(_input_len);
287+
Serial.println(F("# (processInput)-after calling matchCommand()"));
288+
Serial.print(F("#\t_input_index="));Serial.println(_input_index);
289+
Serial.print(F("#\t_input_len="));Serial.println(_input_len);
289290
#endif
290291
if (pcs == PacketShared::SUCCESS){ //a command was matched
291292
#ifdef PACKETCOMMAND_DEBUG
292-
Serial.print(F("(processInput)-matched command: "));
293+
Serial.print(F("# (processInput)-matched command: "));
293294
CommandInfo cmd = getCurrentCommand();
294295
Serial.println(cmd.name);
295296
#endif
296297
}
297298
else if (pcs == PacketShared::ERROR_NO_TYPE_ID_MATCH){ //valid ID but no command was matched
298299
#ifdef PACKETCOMMAND_DEBUG
299-
Serial.println(F("(processInput)-no matched command"));
300+
Serial.println(F("# (processInput)-no matched command"));
300301
#endif
301302
}
302303
else{
@@ -313,20 +314,25 @@ PacketShared::STATUS PacketCommand::processInput(){
313314
PacketShared::STATUS PacketCommand::lookupCommandByName(const char* name){
314315
_current_command = _default_command;
315316
#ifdef PACKETCOMMAND_DEBUG
316-
Serial.print(F("Searching for command named = "));
317+
Serial.println(F("# In PacketCommand::lookupCommandByName"));
318+
Serial.print(F("#\tSearching for command named = "));
317319
Serial.println(name);
318320
#endif
319-
for(size_t i=0; i <= _maxCommands; i++){
321+
for(size_t i=0; i < _maxCommands; i++){
320322
#ifdef PACKETCOMMAND_DEBUG
321-
Serial.print(F("\tsearching command at index="));
323+
Serial.print(F("#\tsearching command at index="));
322324
Serial.println(i);
323-
Serial.print(F("\ttype_id["));Serial.print(_input_index);Serial.print(F("]="));
325+
Serial.print(F("#\t&_commandList[i]="));
326+
Serial.println((int) &_commandList[i], HEX);
327+
Serial.print(F("#\ttype_id["));Serial.print(_input_index);Serial.print(F("]="));
324328
Serial.println(_commandList[i].type_id[_input_index]);
329+
Serial.print(F("#\tname="));
330+
Serial.println(_commandList[i].name);
325331
#endif
326332
if(strcmp(_commandList[i].name,name) == 0){
327333
//a match has been found, so save it and stop
328334
#ifdef PACKETCOMMAND_DEBUG
329-
Serial.println(F("match found"));
335+
Serial.println(F("#\tmatch found"));
330336
#endif
331337
_current_command = _commandList[i];
332338
return PacketShared::SUCCESS;
@@ -389,18 +395,18 @@ PacketShared::STATUS PacketCommand::matchCommand(){
389395
_current_command = _default_command;
390396
//parse out type_id from header
391397
#ifdef PACKETCOMMAND_DEBUG
392-
Serial.println(F("In PacketCommand::matchCommand"));
393-
Serial.print(F("\t_input_index="));Serial.println(_input_index);
394-
Serial.print(F("\t_input_len="));Serial.println(_input_len);
398+
Serial.println(F("# In PacketCommand::matchCommand"));
399+
Serial.print(F("#\t_input_index="));Serial.println(_input_index);
400+
Serial.print(F("#\t_input_len="));Serial.println(_input_len);
395401
#endif
396402
while(_input_index < _input_len){
397403
cur_byte = _input_buffer[_input_index];
398404
#ifdef PACKETCOMMAND_DEBUG
399-
Serial.print(F("cur_byte="));Serial.println(cur_byte,HEX);
405+
Serial.print(F("#\tcur_byte="));Serial.println(cur_byte,HEX);
400406
#endif
401407
if (cur_byte != 0xFF and cur_byte != 0x00){ //valid type ID completed
402408
#ifdef PACKETCOMMAND_DEBUG
403-
Serial.println(F("valid 'type ID' format detected"));
409+
Serial.println(F("#\tvalid 'type ID' format detected"));
404410
#endif
405411
_current_command.type_id[_input_index] = cur_byte;
406412
break;
@@ -410,20 +416,20 @@ PacketShared::STATUS PacketCommand::matchCommand(){
410416
_input_index++;
411417
if (_input_index >= MAX_TYPE_ID_LEN){
412418
#ifdef PACKETCOMMAND_DEBUG
413-
Serial.println(F("Error: invalid 'type ID' detected, exceeded maximum length"));
419+
Serial.println(F("### Error: invalid 'type ID' detected, exceeded maximum length"));
414420
#endif
415421
return PacketShared::ERROR_INVALID_TYPE_ID;
416422
}
417423
else if (_input_index >= _input_len ){ //0xFF cannot end the type_id
418424
#ifdef PACKETCOMMAND_DEBUG
419-
Serial.println(F("Error: invalid packet detected, 'type ID' does not terminate before reaching end of packet"));
425+
Serial.println(F("### Error: invalid packet detected, 'type ID' does not terminate before reaching end of packet"));
420426
#endif
421427
return PacketShared::ERROR_INVALID_PACKET;
422428
}
423429
}
424430
else{ //must be 0x00
425431
#ifdef PACKETCOMMAND_DEBUG
426-
Serial.println(F("Error: invalid 'type ID' detected, cannot contain null (0x00) bytes"));
432+
Serial.println(F("### Error: invalid 'type ID' detected, cannot contain null (0x00) bytes"));
427433
#endif
428434
return PacketShared::ERROR_INVALID_TYPE_ID;
429435
}
@@ -434,17 +440,17 @@ PacketShared::STATUS PacketCommand::matchCommand(){
434440
//since pkt_index must be < MAX_TYPE_ID_LEN at this point, it should be within
435441
//the bounds; and since 'cur_byte' != 0x00 as well, shorter type IDs should
436442
//not match since the unused bytes are initialized to 0x00.
437-
for(size_t i=0; i <= _maxCommands; i++){
443+
for(size_t i=0; i < _maxCommands; i++){
438444
#ifdef PACKETCOMMAND_DEBUG
439-
Serial.print(F("Searching command at index="));
445+
Serial.print(F("# Searching command at index="));
440446
Serial.println(i);
441-
Serial.print(F("\ttype_id["));Serial.print(_input_index);Serial.print(F("]="));
447+
Serial.print(F("#\ttype_id["));Serial.print(_input_index);Serial.print(F("]="));
442448
Serial.println(_commandList[i].type_id[_input_index]);
443449
#endif
444450
if(_commandList[i].type_id[_input_index] == cur_byte){
445451
//a match has been found, so save it and stop
446452
#ifdef PACKETCOMMAND_DEBUG
447-
Serial.println(F("match found"));
453+
Serial.println(F("#match found"));
448454
#endif
449455
_current_command = _commandList[i];
450456
return moveInputBufferIndex(1); //increment to prepare for data unpacking
@@ -453,14 +459,14 @@ PacketShared::STATUS PacketCommand::matchCommand(){
453459
//no type ID has been matched
454460
if (_default_command.function != NULL){ //set the default handler if it has been registered
455461
#ifdef PACKETCOMMAND_DEBUG
456-
Serial.println(F("Setting the default command handler"));
462+
Serial.println(F("# Setting the default command handler"));
457463
#endif
458464
_current_command.function = _default_command.function;
459465
return moveInputBufferIndex(1); //increment to prepare for data unpacking
460466
}
461467
else{ //otherwise return and error condition
462468
#ifdef PACKETCOMMAND_DEBUG
463-
Serial.println(F("No match found for this packet's type ID"));
469+
Serial.println(F("# No match found for this packet's type ID"));
464470
#endif
465471
return PacketShared::ERROR_NO_TYPE_ID_MATCH;
466472
}
@@ -486,15 +492,15 @@ PacketCommand::CommandInfo PacketCommand::getCurrentCommand() {
486492
*/
487493
PacketShared::STATUS PacketCommand::dispatchCommand() {
488494
#ifdef PACKETCOMMAND_DEBUG
489-
Serial.println(F("In PacketCommand::dispatchCommand"));
495+
Serial.println(F("# In PacketCommand::dispatchCommand"));
490496
#endif
491497
if (_current_command.function != NULL){
492498
(*_current_command.function)(*this);
493499
return PacketShared::SUCCESS;
494500
}
495501
else{
496502
#ifdef PACKETCOMMAND_DEBUG
497-
Serial.println(F("Error: tried to dispatch a NULL handler function pointer"));
503+
Serial.println(F("### Error: tried to dispatch a NULL handler function pointer"));
498504
#endif
499505
return PacketShared::ERROR_NULL_HANDLER_FUNCTION_POINTER;
500506
}
@@ -503,6 +509,9 @@ PacketShared::STATUS PacketCommand::dispatchCommand() {
503509
//use unpack* methods to pull out data from packet
504510

505511
PacketShared::STATUS PacketCommand::setupOutputCommandByName(const char* name){
512+
#ifdef PACKETCOMMAND_DEBUG
513+
Serial.println(F("# In PacketCommand::setupOutputCommandByName"));
514+
#endif
506515
PacketShared::STATUS pcs;
507516
pcs = lookupCommandByName(name); //sets _current_command on SUCCESS
508517
//reset output buffer state
@@ -534,7 +543,7 @@ PacketShared::STATUS PacketCommand::send(){
534543
}
535544
else{
536545
#ifdef PACKETCOMMAND_DEBUG
537-
Serial.println(F("Error: tried to send using a NULL send callback function pointer"));
546+
Serial.println(F("### Error: tried to send using a NULL send callback function pointer"));
538547
#endif
539548
return PacketShared::ERROR_NULL_HANDLER_FUNCTION_POINTER;
540549
}
@@ -549,7 +558,7 @@ PacketShared::STATUS PacketCommand::send_nonblocking(){
549558
}
550559
else{
551560
#ifdef PACKETCOMMAND_DEBUG
552-
Serial.println(F("Error: tried to send using a NULL send nonblocking callback function pointer"));
561+
Serial.println(F("### Error: tried to send using a NULL send nonblocking callback function pointer"));
553562
#endif
554563
return PacketShared::ERROR_NULL_HANDLER_FUNCTION_POINTER;
555564
}
@@ -565,7 +574,7 @@ PacketShared::STATUS PacketCommand::reply_send(){
565574
}
566575
else{
567576
#ifdef PACKETCOMMAND_DEBUG
568-
Serial.println(F("Error: tried to send using a NULL send callback function pointer"));
577+
Serial.println(F("### Error: tried to send using a NULL send callback function pointer"));
569578
#endif
570579
return PacketShared::ERROR_NULL_HANDLER_FUNCTION_POINTER;
571580
}
@@ -581,7 +590,7 @@ PacketShared::STATUS PacketCommand::reply_recv(){
581590
}
582591
else{
583592
#ifdef PACKETCOMMAND_DEBUG
584-
Serial.println(F("Error: tried to receive using a NULL recv callback function pointer"));
593+
Serial.println(F("### Error: tried to receive using a NULL recv callback function pointer"));
585594
#endif
586595
return PacketShared::ERROR_NULL_HANDLER_FUNCTION_POINTER;
587596
}
@@ -594,26 +603,26 @@ PacketShared::STATUS PacketCommand::reply_recv(){
594603

595604
PacketShared::STATUS PacketCommand::assignInputBuffer(byte* buff, size_t len){
596605
#ifdef PACKETCOMMAND_DEBUG
597-
Serial.println(F("In PacketCommand::assignInputBuffer"));
598-
Serial.print(F("\tlen="));Serial.println(len);
599-
Serial.print(F("\t_inputBufferSize="));Serial.println(_inputBufferSize);
600-
Serial.print(F("\t_input_index="));Serial.println(_input_index);
601-
Serial.print(F("\t_input_len="));Serial.println(_input_len);
606+
Serial.println(F("# In PacketCommand::assignInputBuffer"));
607+
Serial.print(F("#\tlen="));Serial.println(len);
608+
Serial.print(F("#\t_inputBufferSize="));Serial.println(_inputBufferSize);
609+
Serial.print(F("#\t_input_index="));Serial.println(_input_index);
610+
Serial.print(F("#\t_input_len="));Serial.println(_input_len);
602611
#endif
603612
_input_buffer = buff;
604613
//check the input length before setting
605614
if (len <= _inputBufferSize){
606615
_input_len = len;
607616
#ifdef PACKETCOMMAND_DEBUG
608-
Serial.println(F("(assignInputBuffer) after setting _input_len"));
609-
Serial.print(F("\t_input_index="));Serial.println(_input_index);
610-
Serial.print(F("\t_input_len="));Serial.println(_input_len);
617+
Serial.println(F("# (assignInputBuffer) after setting _input_len"));
618+
Serial.print(F("#\t_input_index="));Serial.println(_input_index);
619+
Serial.print(F("#\t_input_len="));Serial.println(_input_len);
611620
#endif
612621
return PacketShared::SUCCESS;
613622
}
614623
else{
615624
#ifdef PACKETCOMMAND_DEBUG
616-
Serial.println(F("Error: tried to receive data that would overrun input buffer"));
625+
Serial.println(F("### Error: tried to receive data that would overrun input buffer"));
617626
#endif
618627
_input_len = _inputBufferSize; //set to safe value
619628
return PacketShared::ERROR_INPUT_BUFFER_OVERRUN;
@@ -651,15 +660,15 @@ PacketShared::STATUS PacketCommand::moveInputBufferIndex(int n){
651660

652661
void PacketCommand::resetInputBuffer(){
653662
#ifdef PACKETCOMMAND_DEBUG
654-
Serial.println(F("In PacketCommand::resetInputBuffer"));
663+
Serial.println(F("# In PacketCommand::resetInputBuffer"));
655664
#endif
656665
_input_index = 0;
657666
_input_len = 0;
658667
}
659668

660669
PacketShared::STATUS PacketCommand::enqueueInputBuffer(PacketQueue& pq){
661670
#ifdef PACKETCOMMAND_DEBUG
662-
Serial.println(F("In PacketCommand::enqueueInputBuffer"));
671+
Serial.println(F("# In PacketCommand::enqueueInputBuffer"));
663672
#endif
664673
//build a packet struct to hold current buffer state
665674
PacketShared::Packet pkt;
@@ -673,7 +682,7 @@ PacketShared::STATUS PacketCommand::enqueueInputBuffer(PacketQueue& pq){
673682

674683
PacketShared::STATUS PacketCommand::dequeueInputBuffer(PacketQueue& pq){
675684
#ifdef PACKETCOMMAND_DEBUG
676-
Serial.println(F("In PacketCommand::dequeueInputBuffer"));
685+
Serial.println(F("# In PacketCommand::dequeueInputBuffer"));
677686
#endif
678687
//build a packet struct to hold current buffer state
679688
PacketShared::Packet pkt;
@@ -727,7 +736,7 @@ PacketShared::STATUS PacketCommand::moveOutputBufferIndex(int n){
727736

728737
PacketShared::STATUS PacketCommand::enqueueOutputBuffer(PacketQueue& pq){
729738
#ifdef PACKETCOMMAND_DEBUG
730-
Serial.println(F("In PacketCommand::enqueueOutputBuffer"));
739+
Serial.println(F("# In PacketCommand::enqueueOutputBuffer"));
731740
#endif
732741
//build a packet struct to hold current buffer state
733742
PacketShared::Packet pkt;
@@ -741,7 +750,7 @@ PacketShared::STATUS PacketCommand::enqueueOutputBuffer(PacketQueue& pq){
741750

742751
PacketShared::STATUS PacketCommand::dequeueOutputBuffer(PacketQueue& pq){
743752
#ifdef PACKETCOMMAND_DEBUG
744-
Serial.println(F("In PacketCommand::dequeueOutputBuffer"));
753+
Serial.println(F("# In PacketCommand::dequeueOutputBuffer"));
745754
#endif
746755
//build a packet struct to hold current buffer state
747756
PacketShared::Packet pkt;
@@ -766,7 +775,7 @@ PacketShared::STATUS PacketCommand::dequeueOutputBuffer(PacketQueue& pq){
766775
PacketShared::STATUS PacketCommand::requeueOutputBuffer(PacketQueue& pq){
767776
//pushes output buffer onto the front of the queue
768777
#ifdef PACKETCOMMAND_DEBUG
769-
Serial.println(F("In PacketCommand::requeueOutputBuffer"));
778+
Serial.println(F("# In PacketCommand::requeueOutputBuffer"));
770779
#endif
771780
//build a packet struct to hold current buffer state
772781
PacketShared::Packet pkt;

0 commit comments

Comments
 (0)