-
Notifications
You must be signed in to change notification settings - Fork 67
Type Casts for varargs Arguments #237
Description
As mentioned in PR #217, I see potential issues with type casts for arguments that are passed to __VA_ARGS__ functions such as printf. The following example illustrates how the length argument of dtls_debug_hexdump is passed to printf in the RIOTOS-version of tinydtls:
#include <stdio.h>
#define log_write(level, ...) printf(__VA_ARGS__)
enum { LOG_DEBUG };
#define LOG(level, ...) do { \
log_write((level), __VA_ARGS__); } while (0U)
#define LOG_DEBUG(...) LOG(LOG_DEBUG, __VA_ARGS__)
#define dtls_debug_hexdump(name, buf, length) { LOG_DEBUG("(%zu bytes)", (size_t)(length));}
int main(void) {
unsigned short sz = 1 << 15;
dtls_debug_hexdump("", "", sz);
}What happens in a function using __VA_ARGS__ is that the passed arguments are retrieved using va_args that is described for C99 in clause 7.15.1.1 of ISO/IEC 9899:1999 (and pretty much the same for C11):
7.15.1.1 The va_arg macro
Synopsis
#include <stdarg.h>
type va_arg(va_list ap, type);
Description
[...]
The parameter type shall be a type name specified such that the type of a pointer to an object that has the specified type can be obtained simply by postfixing a * to type.
The problem now is that, e.g., the newlib implementation for printf selects a field from a union with a suitable type based on sizeof(size_t)—from (unsigned) short to unsigned long (or even unsigned long long if supported).
So, in the example above, the following lines
unsigned short sz = 1 << 15;
dtls_debug_hexdump("", "", sz);could pretty much cause this:
#define u_quad_t unsigned long
u_quad_t _uquad = va_arg (..., u_quad_t);which, from the description of va_args is semantically equivalent with
unsigned long _uquad = *(unsigned long *)(&sz);Depending on the architecture, this could lead to runtime errors caused by, e.g. accessing illegal addresses or invalid alignment. Unfortunately, it is difficult to construct this situation deliberately, especially on desktop systems rather than embedded platforms.