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.