nothings / stb

stb single-file public domain libraries for C/C++
https://twitter.com/nothings
Other
26.84k stars 7.72k forks source link

stbsp_sprintf infinite while loop #1368

Open juan-carlos-arevalo opened 2 years ago

juan-carlos-arevalo commented 2 years ago

There is an infinite loop in the formatting of leading zeroes in float conversion:

while (tz) { stbsp__int32 i; stbsp__cb_buf_clamp(i, tz); tz -= i; while (i) { if ((((stbsp__uintptr)bf) & 3) == 0) break; *bf++ = '0'; --i; } while (i >= 4) { *(stbsp__uint32 *)bf = 0x30303030; bf += 4; i -= 4; } while (i) { *bf++ = '0'; --i; } stbsp__chk_cb_buf(1); } tz variable is negative then the first or the third loop are infinte thus eventually causing a segfault.

To Reproduce char buff[100]; sprintf(buff, "%0.f", 6, -6, 0.29);

Expected behavior I understand that -6 is a non valid input for trailing zeroes nonetheless, in my opinion should be protected. Maybe by just changing the while condition to i>= 0 it would be fixed. However, I'm not sure if this is an implementation decision for performance

N-R-K commented 2 years ago

I understand that -6 is a non valid input for trailing zeroes nonetheless, in my opinion should be protected.

Depends on what the standard says. If it's undefined then it should probably be assert()-ed as well, which would abort the program on debug builds, instead of silently continuing on as if everything is okay.

Note that this isn't mutually exclusive with fixing the infinite loop, you can fix the loop while still assert()-ing against invalid input which would make debugging easier for the faulty program.

juan-carlos-arevalo commented 2 years ago

Depends on what the standard says. If it's undefined then it should just be it should probably be assert()-ed as well, which would abort the program on debug builds, instead of silently continuing on as if everything is okay.

Note that this isn't mutually exclusive with fixing the infinite loop, you can fix the loop while still assert()-ing against invalid input which would make debugging easier for the faulty program.

Well the standard function just takes the value as if it was not exitent. I am not aware if it is assert()-ed