Function dbus_signature_validate() panics when asked to validate the type signature "a{(ii)i}". According to the docs, dbus_signature_validate() can be used to check a type signature for validity. So, it should indicate the mentioned type signature is invalid. It should not abort the whole program. This was tested on SuSE 11.0, with libdbus 1.2.1. Code to reproduce the error: #include <dbus/dbus.h> int main () { return !dbus_signature_validate("a{(ii)i}", NULL); } Result: process 7016: arguments to dbus_type_is_basic() were incorrect, assertion "_dbus_type_is_valid (typecode) || typecode == DBUS_TYPE_INVALID" failed in file dbus-signature.c line 310. This is normally a bug in some application using the D-Bus library. D-Bus not built with -rdynamic so unable to print a backtrace Aborted
Indeed, this is a bug. (Should add a unit test to avoid this happening again, along with the patch)
The issue seems to be this code that "peeks ahead" at a type code that has not been validated yet: if (last == DBUS_DICT_ENTRY_BEGIN_CHAR && !dbus_type_is_basic (*p)) { result = DBUS_INVALID_DICT_KEY_MUST_BE_BASIC_TYPE; goto out; }
Created attachment 19288 [details] [review] validate type before we evaluate it This looks like it should be sufficient. After a brief study of the function I didn't see any obvious similar problems.
Comment on attachment 19288 [details] [review] validate type before we evaluate it Looks great to me
Thanks for the review, applied: commit 7b10b46c5c8658449783ce45f1273dd35c353bce Author: Colin Walters <walters@verbum.org> Date: Wed Oct 1 13:49:48 2008 -0400 Bug 17803: Panic from dbus_signature_validate * dbus/dbus-marshal-validate.c: Ensure we validate a basic type before calling is_basic on it. * dbus-marshal-validate-util.c: Test.
This flaw has been assigned CVE-2008-3834
Shouldn't a comma be appended to the line preceding the line which is added to dbus/dbus-marshal-validate-util.c? The patch currently does not add a test, but changes the existing last one in the array (the compiler concatenates the string constants).
Ok, so both the test case *and* the validation logic patch were wrong. Wow.
Created attachment 24436 [details] [review] 0001-Bug-17803-Fix-both-test-case-and-validation-logic.patch
John or Havoc, any chance either of you could take a quick look at this updated patch?
This updated flaw has been assigned CVE-2009-1189.
Ouch, I guess it's true you should be sure the test fails before fixing it! (I'm always too lazy myself, but sometimes it bites) The patch looks correct to me, but then, it did last time too...
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.