#8 memory corruption in resolve_rel_url()
There are a couple of insidious memory bugs in
http_client.c:resolve_rel_url() that will cause
memory corruption when using libupnp.so.
Bug #1:
There is an off-by-one error in the allocation of
space for the resolved URL. If your base URL does
not end with a slash ('/'),
e.g. "http://10.19.13.136:543" then the result
will occupy one extra byte beyond that malloced at
the start of the routine. The actual source of
the problem is that a slash will be added between
the base_url and the rel_url, and that extra
character is not accounted for in the call to
malloc
The fix is simple - the malloc call should read,
out = (char *) malloc(strlen(base_url) + strlen
(rel_url) + 2);
Bug #2:
resolve_rel_url() calls free() on its second
argument. That's a *really* bad idea, and
apparently that was recognized because a comment
in upnptools.c:UpnpResolveURL() explicitly
mentions the desire to fix it.
In any case the code in
upnptools.c:UpnpResolveURL() deals with the
problem by allocating memory and copy its data
before calling resolve_rel_url().
The code in service_table.c:getServiceList(),
however, does not. It calls resolve_rel_url() and
continues to use the data that resolve_rel_url()
has just freed.
Clearly the answer is to fix resolve_rel_url() to
not free it second argument. Then you need to
remove the extra code from
upnptools.c:UpnpResolveURL() since it no longer
needs to malloc space for and copy its data.
As far as I can tell the memory corruption caused
by resolve_rel_url() is not detectable in the
tvdevice sample but it was imemdiately apparent
when I tried to use the Intel SDK as the basis of
a sample internet gateway device.
I've attached a new version of resolve_rel_url()
and UPNPResolveURL() that fixes the problems.
suggested fixes for resolve_rel_url bugs.
Logged In: NO
Whoops, I was mistaken about one thing - the code that
calls resolve_rel_url() from service_table.c:getServiceList
() did assume that resolve_rel_url() would free its second
argument. The call is screwy though since it does no error
checking on the return value from resolve_rel_url() so
there is some change that the argument will not be free'd.
Nonetheless, the right thing to do is still to not free the
argument in resolve_rel_url(), under the basic primciple
that memory should be freed close to where it is
allocated. I'm going to rewrite getServiceList to take
care of this and if anyone wants the rewritten version,
drop me a line and I'll be happy to send it.