On 02/05/2010 06:12 AM, Luiz Capitulino wrote:
On Thu, 04 Feb 2010 16:31:46 -0600
Anthony Liguori<anth...@codemonkey.ws>  wrote:

On 02/04/2010 02:13 PM, Luiz Capitulino wrote:
Add an assert() to qobject_from_jsonf() to assure that the returned
QObject is not NULL. Currently this is duplicated in the callers.

Signed-off-by: Luiz Capitulino<lcapitul...@redhat.com>
---
   qjson.c |    1 +
   1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qjson.c b/qjson.c
index 9ad8a91..0922c06 100644
--- a/qjson.c
+++ b/qjson.c
@@ -62,6 +62,7 @@ QObject *qobject_from_jsonf(const char *string, ...)
       obj = qobject_from_jsonv(string,&ap);
       va_end(ap);

+    assert(obj != NULL);

This is wrong.  We may get JSON from an untrusted source.  Callers need
to deal with failure appropriately.
  What kind of untrusted source? This function is only used by handlers
and assuming that the only possible error here is bad syntax, not having
this check in the source will only duplicate it in the users.

I don't know yet, but there's nothing about this function that indicates that it cannot handle malformed JSON. I don't think it's a reasonable expectation either.

There are absolutely ways to mitigate this. You can use GCC macros to enforce at compile time that the string argument is always a literal and never a user supplied string.

Run time asserts are a terrible way to deal with reasonably expected errors.

Regards,

Anthony Liguori


Reply via email to