[rrd-developers] Dereferencing of NULL-pointer in rrd_restore.c (patch attached)

[Florian Forster]

Hi everybody,

I tripped over this bug in rrd_restore.c, function xml2rrd:
-- 8< --
 eat_tag(&ptr2, "params");
 skip(&ptr2);
-- >8 --
This is problematic, because `eat_tag' sets `ptr2' to NULL if the tag
cannot be found, and `skip' dereferences `ptr2' without checking it
first.

The attached patch makes `xml2rrd' honor the return value and makes
`skip' check it's arguments. I have checked the rest of the code only
very superficially, so there may be similar problems to the one outlined
above. To be quite honest, by the look of the code I'd be surprised if
there wasn't.

Have you ever thought about using an XML-library to parse the input? (I
searched the archives but couldn't find anything appropriate.) I bet the
code would get a lot smaller and easier to maintain. Also it'd probably
be possible to get rid of the need to have the tags in a given order.
(That's what caused the segfault in the first place and bit me in the
leg just now..)

Regards,
-octo
--
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/
-------------- next part --------------
diff -pur rrdtool-1.2.19.orig/src/rrd_restore.c rrdtool-1.2.19.octo/src/rrd_restore.c
--- rrdtool-1.2.19.orig/src/rrd_restore.c	2007-02-01 06:51:38.000000000 +0100
+++ rrdtool-1.2.19.octo/src/rrd_restore.c	2007-04-01 15:55:19.096267356 +0200
@@ -65,6 +65,8 @@ int skipxml(char **buf){
 
 int skip(char **buf){
   char *ptr;  
+  if ((buf == NULL) || (*buf == NULL))
+    return -1;
   ptr=(*buf);
   do {
     (*buf)=ptr;
@@ -254,7 +256,7 @@ int xml2rrd(char* buf, rrd_t* rrd, char 
          read_tag(&ptr2, "xff","%lf",
             &(rrd->rra_def[rra_index].par[RRA_cdp_xff_val].u_val));
       } else {
-        eat_tag(&ptr2, "params");
+        if (eat_tag(&ptr2, "params") != 1) return -1;
         skip(&ptr2);
         /* backwards compatibility w/ old patch */
       if (strncmp(ptr2, "<value>",7) == 0) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.oetiker.ch/pipermail/rrd-developers/attachments/20070402/71c7effa/attachment-0001.pgp