reposting for the day crowd: I ran into a memcmp implementation that only compared 25% of the bytes, and the issue wasn't caught in the build because the vendor toolchain failed to emit a warning.
-
to be fair it should also have been unit tested but I'm gonna cut the devs some slack here because the toolchain vendor rugpulling a whole warning category is a significantly worse offense.
@gsuberland Pretty sure this would have passed the unit tests that anyone would have been likely to write anyway.
-
@gsuberland Pretty sure this would have passed the unit tests that anyone would have been likely to write anyway.
@WAHa_06x36 this is why fuzz testing is a thing!
-
@WAHa_06x36 this is why fuzz testing is a thing!
@gsuberland Hmm, would even fuzz testing find it? That seems tricky to set up in a way that a) would actually find the bug and b) would occur to you before seeing the bug.
I guess for very short inputs you might find it more easily by chance...
-
@gsuberland Hmm, would even fuzz testing find it? That seems tricky to set up in a way that a) would actually find the bug and b) would occur to you before seeing the bug.
I guess for very short inputs you might find it more easily by chance...
@WAHa_06x36 of course. fuzz testing would quickly find memcmp("aaaa", "Aaaa") == 0 or memcmp("aaaa", "aaaA") == 0 as a violation of the contract (depending on endianness)
-
@gsuberland Hmm, would even fuzz testing find it? That seems tricky to set up in a way that a) would actually find the bug and b) would occur to you before seeing the bug.
I guess for very short inputs you might find it more easily by chance...
@WAHa_06x36 @gsuberland i think „only one byte differs“ kind of tests would probably find it, right? And these seem like something you’d write to test that
-
@gsuberland Fair. You should add clang as well... and please add that you need to use -Wconversion
@gsuberland @uecker I won’t defend Clang’s naming choices in every case, but I believe this specific one is all GCC; Clang originally called this -Wc++0x-narrowing (eventually -Wc++11-narrowing) and only added the -Wnarrowing alias for GCC compatibility. In any case, the documentation should really suggest -Wconversion, and on that front I can definitely accept blame for Clang, because our warning group documentation is awful
-
@gsuberland @uecker I won’t defend Clang’s naming choices in every case, but I believe this specific one is all GCC; Clang originally called this -Wc++0x-narrowing (eventually -Wc++11-narrowing) and only added the -Wnarrowing alias for GCC compatibility. In any case, the documentation should really suggest -Wconversion, and on that front I can definitely accept blame for Clang, because our warning group documentation is awful
-
@gsuberland @rjmccall It seems it is under the language dialects options and explanation is not really clear. https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/C_002b_002b-Dialect-Options.html
-
@gsuberland @rjmccall It seems it is under the language dialects options and explanation is not really clear. https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/C_002b_002b-Dialect-Options.html
-
@WAHa_06x36 of course. fuzz testing would quickly find memcmp("aaaa", "Aaaa") == 0 or memcmp("aaaa", "aaaA") == 0 as a violation of the contract (depending on endianness)
@gsuberland I mean, if you set up a special test harness against a known-good implementation and used something like afl that actually instruments the code itself, maybe, but, who would ever do that?
-
@gsuberland I mean, if you set up a special test harness against a known-good implementation and used something like afl that actually instruments the code itself, maybe, but, who would ever do that?
@WAHa_06x36 quite a few people! there are even coverage tools specifically for doing this.
-
@WAHa_06x36 quite a few people! there are even coverage tools specifically for doing this.
@gsuberland Hmm, interesting, haven't seen those!
-
reposting for the day crowd: I ran into a memcmp implementation that only compared 25% of the bytes, and the issue wasn't caught in the build because the vendor toolchain failed to emit a warning.
@gsuberland that seems not good
-
R relay@relay.infosec.exchange shared this topic
