On 29 Mar 2005 Daniel THOMPSON <daniel.thompson@xxxxxx> wrote:
Hi,
> There are routes through tComponent::FromString() that explicitly set
> description to NULL without checking its value first (when n != 4). This
> appears to me to be the leak.
>
> Running the following code it is obvious that glibc malloc'ed desc even
> when desc is not converted.
Wow, this is cool. How did you get the idea to search in that
direction?
Anyways, I think the man page isn't very clear on this fact...
> Thus the simpler fix for this leak, and one that is not prone to buffer
> overflow, is:
>
> --- vdr-1.3.22/epg.c.orig 2005-03-29 09:03:23.484024000 +0100
> +++ vdr-1.3.22/epg.c 2005-03-29 09:04:16.359024000 +0100
> @@ -29,9 +29,7 @@
> {
> unsigned int Stream, Type;
> int n = sscanf(s, "%X %02X %3c %a[^\n]", &Stream, &Type, language,
> &description);
> - if (n != 4)
> - description = NULL;
> - else if (isempty(description)) {
> + if (n != 4 || isempty(description)) {
> free(description);
> description = NULL;
> }
>
> While that are more robust ways to handle the case then desc is NULL
> (which for my glibc it never is) the above code is safe since both
> isempty() and glibc's free() can safely handle NULL pointers.
What about this?
if (description!=NULL && (n != 4 || isempty(description)))
Regards.
--
Stefan Huelswitt
s.huelswitt@xxxxxx | http://www.muempf.de/
