Solar Designer <[email protected]> writes: > Thank you, Justin! > > In cases like this, we should be bringing the entire report to > oss-security, not just a link. So I'll include it below. > > Further in the above thread, there's a link to a fix pull request by > Collin Funk. I didn't review it in full context, but even within the > patch context it fails my review: > > add_slc (char func, char flag, cc_t val) > { > > /* Do nothing if the entire triplet cannot fit in the buffer. */ > if (slcbuf + sizeof slcbuf <= slcptr + 6) > return; > > if ((*slcptr++ = (unsigned char) func) == 0xff) > *slcptr++ = 0xff; > > if ((*slcptr++ = (unsigned char) flag) == 0xff) > *slcptr++ = 0xff; > > if ((*slcptr++ = (unsigned char) val) == 0xff) > *slcptr++ = 0xff; > > } /* end of add_slc */ > > In "slcptr + 6", it appears to rely on pointer math working outside of > the object, but that's UB in C. If the C compiler concludes that the > "if" condition cannot be true within defined behavior, it is free to > optimize the entire "if" and "return" out.
CC'ing bug-gnulib. Do we make any assumptions about this behavior in Gnulib? I know we generally assume systems are more sane than ISO C requires. E.g. no holes in integers, flat address space, etc. Perhaps it is worth another bullet point in our documentation [1]. > A proper check may be: > > if (slcbuf + sizeof slcbuf - 6 <= slcptr) > > or perhaps with "<" in place of "<=", unless we need an extra element > for some reason. Yeah, that works. Thanks. Collin [1] https://www.gnu.org/software/gnulib/manual/gnulib.html#Other-portability-assumptions-made-by-Gnulib
