Several memory issues in Uduino's Arduino code:

Hello,

my company is currently working on an installation with several led stripes, and the communication between the led-controlling Arduino and our Unity app is implemented using Uduino. Oftentimes we noticed some strange behavior on the Arduino's side with commands not executing properly and sometimes completely random behaviour. I managed to pin down most issues on some bugs within the Uduino.cpp/.h arduino code:

‌

1. Delimiter-string is not null terminated

In Uduino.cpp, lines 141, 191 and 228, commands are parsed using strtok_r like this:

line 141: nextToken = strtok_r(NULL, delimBundle, &last);

However, delimBundle is initialized like this:

 line 45: strncpy(delimBundle," ,",MAXDELIMETER);

With MAXDELIMETER being 2, delimBundle is not longer \0-terminated, which causes strtok_r to continue searching the memory for more random bytes which it may then use as delimiters. This causes some commands and arguments sent via serial to be parsed completely different.

‌

2. Memory corruption in addCommand function

When adding commands using the AddCommand()-function, commands are stored using:

line 337: strncpy(CommandList[numCommand].command,command,UDUINOBUFFER);

However, each .command-chararray is initialized with length COMMANDMAXNAME, which is 16 by default (UDUINOBUFFER is 128). Per the documentation, strncpy always writes n bytes (128 in this case), with the last bytes being zeroes if the source string is shorter than n. In this case it causes random blocks of memory behind the Uduino object (and also members of the uduino-class like defaultHandler) to be overwritten with zeroes. Also note that strncpy does not terminate strings with \0-bytes if the source string is as long or longer than n bytes.

‌

3. Just a nitpick, but:

272: buffer[bufPos++]=inChar;   // Put character into buffer
273: buffer[bufPos]='\0';  // Null terminate
274: if (bufPos > UDUINOBUFFER-1) bufPos=0; // wrap buffer around if full

Since buffer is UDUINOBUFFER btes long, the last element is UDUINOBUFFER-1. But in line 273, element UDUINOBUFFER may be set to zero, which is out of range. Funny enough, at that memory address is usually the bufpos-variable, which would be otherwise set to 0 in line 274, so this bug kinda cancels itself out.

Comments (1)