Re: minidlna buffer overflow on netbsd-8/amd64

[maya%NetBSD.org@localhost]

On Sat, Oct 26, 2019 at 10:46:56PM +0000, maya%NetBSD.org@localhost wrote:
> On Sat, Oct 26, 2019 at 04:47:40PM -0500, John D. Baker wrote:
> > For some time now I've seen meessages like the following on the machine
> > that acts as DLNA server (also my file server) whenever I reboot it (and
> > start minidlna from my "rc.local" script):
> > 
> >   Oct 25 15:14:26 hostname -: hostname.domainname.tld minidlnad - - - buffer overflow detected; terminated
> > 
> > As a result, while there is still a minidlnad process present and the
> > minidlna server is visible to DNLA clients, no content is listed.
> > Top-level directories are there, but no contents or subdirectories.
> > 
> > I've been starting it with the "-R" switch so it will re-index the
> > contents so that recently-added files will be visible.  (Before it
> > started complaining about the overflow, It didn't seem to pick up new
> > files by itself unless restarted with -R).
> > 
> > Has anyone seen anything like this before?  Have I hit some limit of
> > minidlna in terms of the number of files or something like that?
> 
> This failure is from stack protector.
> https://nxr.netbsd.org/xref/src/lib/libc/misc/stack_protector.c#107
> 
> I guess if you really wanted it to work right away, you could add to the
> package makefile:
> SSP_SUPPORTED=no
> 
> Stack protector is writing an unpredictable random value to the stack
> and checking that the random value is the same later.
> 
> Failing this check means something overflows a buffer, and might be a
> big security issue.
> 
> So far, in regular userland programs like minidlna, every time I had a
> failure from SSP it was either an indication of very problematic code or
> security issues. So I would hesitate to continue running it without an
> investigation.

As a starting point, it uses MAXPATHLEN as a buffer size. I think it
should be MAXPATHLEN+1 (or PATH_MAX) to avoid an overflow with the null
byte.
(Highly unlikely this could be abused).

        char file[MAXPATHLEN];
...
                strncpyt(file, path, sizeof(file));
                p = strrchr(file, '.');
                if( p )
                {
                        strcpy(p, ".jpg");
                        ret = access(file, R_OK);
                }
                if( ret != 0 )

Will overflow the file buffer if it's a really long pathname and it ends
with, say, /really/long/path.

This is also repeated in check_for_captions (at least we made a utility
function for it there).


It looks like all the calls to strip_ext that don't use the return value
are pointless. It should also constify the input to make it slightly
more obivous.

I suspect that the sql.*%s results allow for SQL injection. I wonder
whether it can be blindly substituted with %q. I don't sqlite much.

Would directory traversals be a bug in minidlna?