...
In this noncompliant code example, length
is the value of a user-supplied argument that defined (and thus potentially untrusted) environment variable whose value is used to determine the length size of a dynamically allocated array, table
. In compliance with INT30-C. Ensure that unsigned integer operations do not wrap the code prevents unsigned integer wrapping but it doesn't impose any upper bound on the size of the array, making it possible for the user to cause the program to use an excessive amount of memory.
Code Block | ||
---|---|---|
| ||
intchar** create_table(void) { const char* const lenstr = getenv("TABLE_SIZE"); const size_t length) { char **table = lenstr ? strtoul(lenstr, 0, 10) : 0; if (length > SIZE_MAX / sizeof(char *)) { return /*NULL; handle overflow /*/ indicate error to return -1;caller */ } const size_t table_lengthsize = length * sizeof(char *); char** const table = (char **)malloc(table_lengthsize); if (table == NULL) { /*return HandleNULL; error condition /*/ indicate error to return -1; }caller */ /* initialize table... */ return 0table; } |
Because length
is user controlled, the value can result in a large block of memory being allocated or can cause the call to malloc()
to fail. Depending on how error handling is implemented, this may result in a denial of service or other error. A length
of zero results in a division by zero in the overflow check, which can also result in a denial of service. (See rule INT33-C. Ensure that division and modulo operations do not result in divide-by-zero errors.)
Compliant Solution
Wiki Markup |
---|
This compliant solution defines the acceptable range for {{length}} as {{\[1, MAX_TABLE_LENGTH\]}}. The {{length}} parameter is declared as {{size_t}}, which is unsigned by definition. Consequently, it is not necessary to check {{length}} for negative values. (See recommendation [INT01-C. Use rsize_t or size_t for all integer values representing the size of an object].) |
Code Block | ||
---|---|---|
| ||
enum { MAX_TABLE_LENGTH = 256 }; intchar** create_table(size_t lengthvoid) { size_t table_length; const char **table; const if (lengthlenstr == 0 || length > MAX_TABLE_LENGTH) {getenv("TABLE_SIZE"); const size_t /*length Handle= invalidlenstr length */ return -1; } /* * The wrap check has been omitted based on the assumption * that ? strtoul(lenstr, 0, 10) : 0; if (length == 0 || length > MAX_TABLE_LENGTH * sizeof(char *) cannot exceed * SIZE_MAX. If this assumption is not valid, a check must * be added. return NULL; /* indicate error to caller */ assert(length <= SIZE_MAX/sizeof(char *)); table_lengthconst size_t table_size = length * sizeof(char *); char** const table = (char **)malloc(table_lengthsize); if (table == NULL) { /*return HandleNULL; error condition /*/ indicate error to return -1; }caller */ /* initialize table... */ return 0table; } |
The test for length == 0
ensures that a nonzero number of bytes is allocated. (See recommendation MEM04-C. Do not perform zero length allocations.)
...