GNOME Bugzilla – Bug 531236
[0.11][gstvalue] Deserialization not correct
Last modified: 2013-07-17 11:08:02 UTC
Hi, the deserialization in gstvalue.c is not correct in two places: a) gst_value_deserialize_int_helper There's *to = g_ascii_strtoull (s, &end, 0), which only works on unsigned strings. Later on the return value of this is even checked if it's smaller than zero ;) IMHO this should be _strtoll() instead. Am I missing something? Also there should probably be a check that checks if the deserialized value is in the [min,max] range... b) gst_value_deserialize_u* Here the _unsigned_ result of g_ascii_strtoull() is stored in a gint64, thus things like overflows can happen easily. Checks if the value is between [min,max] should be added here too I guess... Is my analysis correct? If so I'll prepare a patch later...
Ok, I'm looking closer at it now and there are actually a few problems with that code... will explain later with a patch :)
Ok, so we're currently deserializing "-1" into unsigned integer types and allow deserializing out of range values into several integer types. Fixing this might break other things so we should probably wait for this until 0.11 and clean everything up and make it more strict then. It might make sense to use a copy of GLib's internal g_parse_long_long() for this as it will give the magnitude of values and it's sign so we can reliably detect all kind of weird things that happen because of overflows currently.
> Ok, I'm looking closer at it now and there are actually a few problems with > that code... will explain later with a patch :) Four years, and we're still waiting for that patch ;)
Sebastian: now is the time - fix or WONTFIX please :-) A unit test that shows the problem would be helpful too.
Let's just close it, there is no real bug caused by this and code is depending on the current behaviour.