I am working with some legacy C code running on a Raspberry Pi (3, I think) running arch linux. As part of the app's start-up, it reads in a file line by line and stores each line to some custom structure.
99 times out of a hundred, this works fine and we all live happily ever after. In that odd one out, I get a stack smashing detected error, forcing a reset.
I've already verified that the lines are correct and consistently formatted.
I've narrowed it down to another function that gets called twice during the file parsing:
const char* num = "+-.0123456789";
const char* Mults = "pnumkMGT";
float engtof(char* s)
{
printf("engtof: %s\n",s);
float f, g = 1;
char m, *q = 0, w[32] = {0};
int i;
for(i = 0; i < 32; i++) w[i] = 0; // clear w
strcpy(w, s);
for(q = w; strchr(num, *q) != NULL; q++);
puts(q); // sometimes prints some garbage after the desired char
if(*q == 'E' || *q =='e')
return atof(w);
if(*q != 0 && strchr(Mults, *q) != NULL)
{
m = *q;
g = getMult(m); // behavior already verified
*q = 0;
}
puts(w); // seems to reach here before sometimes stack smashing, but w still looks right...
f = atof(w);
f *= g;
return f;
}
(Disregard the various printf and puts statements. Those are there for debugging, since we can't figure out how to get remote debugging from NetBeans 12.3 working)
This is supposed to take a string in engineering format (s = "{value}{multiplier for power of 10}{unit}") (s="xkV" or s="a.bcmA", for example) and convert it to a float value (x000 or 0.00abc in the examples). This gets called ~300 times during startup when everything works, but when it doesn't work we stack smash on the first call (usually with s = "0").
Everything I've found so far on stack smashing talks about buffer overflow and out-of-range indices, but as far as I can tell, neither of those apply here. Granted, my understanding of pointers is only so-so...
If it happened every time, I'm confident I could find the problem, but since it happens so rarely, I'm at a loss on why this is happening. Anyone have any insight on this?
EDIT: Here's the function that's calling engtof during startup:
void loadCalFactors()
{
printf("loadCalFactors\n");
FILE *fp;
char s[120], w[80] = {0}, *p;
int i, j;
fp = fopen(fileCF, "r");
if(fp == NULL)
{
sprintf(s, "Can't open: %s\n", fileCF);
printf(s);
netWrite(s);
return;
}
for(i = 0; i < N_CALFACTOR; i++)
{
j = 0;
fgets(s, 80, fp);
//puts(s);
p = strtok(s, ",");
while(p != NULL)
{
//puts(p);
switch(j)
{
case 0:
//puts("Header");
strcpy(w, p);// header
j++;
break;
case 1:
//puts("Gain");
calFactors[i].g = engtof(p);
j++;
break;
case 2:
//puts("Offset");
calFactors[i].os = engtof(p);
j++;
break;
case 3:
//puts("Unit");
strcpy(calFactors[i].unit, p);
j++;
break;
}
//printf("j= %i, p= %s\n",j,p);
p = strtok(NULL, ",");
}
}
fclose(fp);
puts("loadCalFactors done");
}
It only ever seems to have a problem with the first call to engtof. If it gets past that, it goes all the way through the file just fine.
Here are the first few lines of fileCF:
Input Source 5V Range,0,0,V
Input Voltmeter 5V Range,0,0,V
Input Source -5V Range,0,0,V
Input Voltmeter -5V Range,0,0,V
Input Source 50V Range,0,0,V
Input Voltmeter 50V Range,0,0,V
Input Source -50V Range,0,0,V
Input Voltmeter -50V Range,0,0,V
And here's getMult:
float getMult(char m)
{
puts("getMult");
float f;
printf("m=%c\n",m);
switch(m)
{
case 'p': f = 1E-12; break;
case 'n': f = 1E-9; break;
case 'u': f = 1E-6; break;
case 'm': f = 1E-3; break;
case 'k': f = 1E3; break;
case 'M': f = 1E6; break;
case 'G': f = 1E9; break;
case 'T': f = 1E12; break;
default: f = 1; break;
}
//printf("f=%g\n",f);
return f;
}
In the interest of helping produce a MVE, here's all our includes:
#include <fcntl.h>
#include <errno.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/poll.h>
#include <netinet/in.h>
#include <netdb.h>
#include <unistd.h>
#include <time.h>
#include <wiringPi.h>
#include <math.h>
#include <ctype.h>
I don't know how many of these are even used in the sections shared above, and I'm sure none of them are being used to their full potential.
The line
for(q = w; strchr(num, *q) != NULL; q++);
will invoke undefined behavior if you call the function with s pointing to the string "0". This loop will only terminate once q points to a non-null character that is not in num, which will not be the case until q points out of bounds of w.
If you want q to point to the first character in w that is not part of num, then you can use the following line instead:
q = w + strspn( w, num );
It is also possible to solve this (less efficiently) by calling strchr in a loop, as you have attempted. However, the correct way to do this would be:
for(q = w; *q != '\0' && strchr(num, *q) != NULL; q++)
;
Adding the expression *q != '\0' will prevent the pointer q from going out of bounds of w (assuming that the string has a null terminating character).