From ea192f08e609fa4c4a48df1b27874b9ae2c1fa40 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 14 Feb 2009 18:01:20 +0100 Subject: Fix an invalid typecasting entry->num_of_strings is a uint16_t. Casting it with (int *)&entry->num_of_strings is wrong, because it gives add_string_to_array the illusion that the object "num" points to is an int, which it is not. In case we are running on a machine where "int" is 32 or 64 bits long, what happens with that cast? "add_string_to_array" interprets the byte field that starts where "num_of_strings" starts as an int. Under very particular circumstances this might work in a limited number of cases: When the byte order of an int is such that the lower order bits of the int are stored first, the subsequent bytes which do not belong to the uint16_t anymore happen to be 0 and the result of the increment still fits into the first 2 bytes of that int, i.e. the result is < 65536. The correct solution to this problem is to use the implicit type conversion that happens when an assignment is done. BTW, this bug is found if you compile with -O3 -Wall, it shows up as a warning: rpc_server/srv_eventlog_lib.c:574: warning: dereferencing type-punned pointer will break strict-aliasing rules Thanks, Volker --- source3/rpc_server/srv_eventlog_lib.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'source3/rpc_server') diff --git a/source3/rpc_server/srv_eventlog_lib.c b/source3/rpc_server/srv_eventlog_lib.c index d8c5c3d453..edd1cfacb8 100644 --- a/source3/rpc_server/srv_eventlog_lib.c +++ b/source3/rpc_server/srv_eventlog_lib.c @@ -560,6 +560,7 @@ bool parse_logentry( TALLOC_CTX *mem_ctx, char *line, struct eventlog_Record_tdb } } else if ( 0 == strncmp( start, "STR", stop - start ) ) { size_t tmp_len; + int num_of_strings; /* skip past initial ":" */ stop++; /* now skip any other leading whitespace */ @@ -570,10 +571,15 @@ bool parse_logentry( TALLOC_CTX *mem_ctx, char *line, struct eventlog_Record_tdb if (tmp_len == (size_t)-1) { return false; } + num_of_strings = entry->num_of_strings; if (!add_string_to_array(mem_ctx, stop, &entry->strings, - (int *)&entry->num_of_strings)) { + &num_of_strings)) { return false; } + if (num_of_strings > 0xffff) { + return false; + } + entry->num_of_strings = num_of_strings; entry->strings_len += tmp_len; } else if ( 0 == strncmp( start, "DAT", stop - start ) ) { /* skip past initial ":" */ -- cgit