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

diff --git a/source3/rpc_server/srv_eventlog_lib.c b/source3/rpc_server/srv_eventlog_lib.c
index d8c5c3d4539b62f6857ea60a8ecc4bbb93ba8f7b..edd1cfacb829a1fe9638a646656a87b852cccd2d 100644 (file)
--- 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 ":" */