Ok, I've had a closer look at this. It turns out there was a bug in get_number():
Code:
diff --git a/src/handler.c b/src/handler.c
index 87269bb..9aeb664 100644
--- a/src/handler.c
+++ b/src/handler.c
@@ -592,14 +592,16 @@ int get_number(char **name)
{
int i;
char *ppos;
- char number[MAX_INPUT_LENGTH];
+ char number[MAX_INPUT_LENGTH], tmp[MAX_INPUT_LENGTH];
*number = '\0';
if ((ppos = strchr(*name, '.')) != NULL) {
*ppos++ = '\0';
strlcpy(number, *name, sizeof(number));
- strcpy(*name, ppos); /* strcpy: OK (always smaller) */
+ // avoid overlapping strings in strcpy which is undefined behaviour
+ strcpy(tmp, ppos); /* strcpy: OK (always smaller) */
+ strcpy(*name, tmp); /* strcpy: OK (always smaller) */
for (i = 0; *(number + i); i++)
if (!isdigit(*(number + i)))
The copy is happening from *name+2 -> *name and that's an overlap that means undefined behavior in strcpy.
This has made me introduce unit testing, because this worked locally, but fortunately failed on the github build.
Feel free to come with feedback on
github.com/tbamud/tbamud/pull/134
