Handle zero-length SYSEX messages

[mchesser]

Simple fix for #151. Check that the storedInputData buffer is not empty before reading the command ID from the buffer.

[pgrawehr]

Thanks, this is looking good. Just curious: Is there any reasonable use case where this happens? Or did you find this just based on a bug in the client library?

[echavet]

De mémoire et en ayant jeté un oeil rapide, sauf erreur ce cas n'est pas possible. La fonction n'est appelée que lorqu'un message sysex complet est déjà dans le tampon.
Mais bon au cas où on l'appellerait en dehors du contexte actuel, la modif est bonne.

[mchesser]

Thanks, this is looking good. Just curious: Is there any reasonable use case where this happens? Or did you find this just based on a bug in the client library?

The actual issue was originally found via some fuzzing experiments I was doing. But I was envisioning an API like SendSysex(data) that automatically adds SYSEX_START/SYSEX_END to the data, that could be accidentally misused by providing an empty buffer to the function. I also thought it might be possible for a byte to be dropped because line corruption. Since this issue is somewhat difficult to debug, I thought it would be nice to fix.

I had quick look through the other implementations, and the .NET IoT implementation does contain a similar bytes_read check when parsing SYSEX messages FirmataDevice.cs:359-364

De mémoire et en ayant jeté un oeil rapide, sauf erreur ce cas n'est pas possible. La fonction n'est appelée que lorqu'un message sysex complet est déjà dans le tampon. Mais bon au cas où on l'appellerait en dehors du contexte actuel, la modif est bonne.

If the bytes [START_SYSEX, END_SYSEX] are sent, for the first byte the following code is executed: ConfigurableFirmata.cpp:407-410, for the second byte the code here is executed: ConfigurableFirmata.cpp:322-326. Neither of those two blocks of code ever write to storedInputData so storedInputData is effectively empty (the prefix/suffix bytes are never written to the buffer). Another option is to move the check to Line 326 instead.

_(J'ai dû recourir à Google Translate pour répondre, désolé si c'est difficile à comprendre), Si les octets [START_SYSEX, END_SYSEX] sont envoyés : pour le premier octet le code suivant est exécuté : ConfigurableFirmata.cpp:407-410, pour le deuxième octet, le code ici est exécuté : ConfigurableFirmata.cpp:322-326. Aucun de ces deux blocs de code n'écrit jamais dans storeInputData, donc storeInputData est effectivement vide (les octets de préfixe/suffixe ne sont jamais écrits dans le tampon). Une autre option consiste à déplacer le chèque vers Line 326

[pgrawehr]

There appears to be a compatibility issue with the ESP IDK 3.0. I need to look into that first.

[pgrawehr]

@mchesser Can you please rebase your PR? That should now fix the build.

[mchesser]

Rebased.

[pgrawehr]

Thanks again. Merged.