-
-
Notifications
You must be signed in to change notification settings - Fork 67
Open
Description
In at least a bunch of tests, we should check that we never write outside of the provided target buffer.
We should check for:
- strings that are longer than the buffer (should properly 0-terminate, and return the correct as-if-no-overrun length)
- with long input strings to %s
- with large widths
- with large precisions
- with multiple conversion specifiers that each produce short strings, that only overflow when concatenated
- with long stretches of literal characters in the format string
Also, right now most or all tests introduce a NUL character at the end of the output buffer, by something like:
npf_vsnprintf(buf, sizeof(buf), fmt, args);
buf[sizeof(buf)-1] = '\0';
Instead, we should first check that the buffer is indeed terminated, with:
int len = strnlen(buf, sizeof(buf));
REQUIRE(len < sizeof(buf));
which ensures that we never read past the buffer, but we also detect whether there is a terminator.
Then, we can add the terminator ourselves, as already done, so that subsequent checks produce nice error messages instead of crashes.
The check that NPF does not write outside the buffer can be something like this (used similarly to the current require_conform
):
void check_string(const char* expected, char const *fmt, ...) {
#define BUF_PAD 16
#define BUF_SZ 256
#define FILLER 0xFF
char buf[BUF_PAD + BUF_SZ + BUF_PAD];
memset(buf, FILLER, sizeof(buf));
int n;
{
va_list args;
va_start(args, fmt);
n = npf_vsnprintf(buf + BUF_PAD, BUF_SZ, fmt, args);
va_end(args);
}
int len = strnlen(buf + BUF_PAD, BUF_SZ);
buf[BUF_PAD + BUF_SZ - 1] = '\0';
REQUIRE(len < BUF_SZ);
REQUIRE(n == len);
if (n < BUF_SIZ) { // guards against unterminated strings, which might cause this very test code to cause overrun
// clear the chars written by printf (plus the, and check that no other were overwritten
memset(buf + BUF_SZ, FILLER, n+1); // +1: also clear the NUL terminator
REQUIRE(is_filled(buf, FILLER, sizeof(buf);
}
}
bool is_filled(char* b, char value, size_t n_bytes)
{
// Check for any non-matching value in the array.
// Since all-equal is the common case, we optimize for that, by not having
// an early-bailout branch in the loop. We always access all the elements,
// but that's what happens in the common case too.
// (a ^ b) is all-0 iff (a == b)
// OR-ing that into 'res' means that we accumulate any error into 'res' (it
// will get a sticky non-0 bit). At the end, they were all equal iff (res==0)
unsigned res = 0;
for(size_t i = 0; i < n_bytes; ++i) {
res |= (unsigned)b[i] ^ (unsigned)value;
}
return !res;
}
Metadata
Metadata
Assignees
Labels
No labels