Mail archive
alpine-aports

Re: [alpine-aports] [PATCH] community/minidlna: patch for potential segfaults

From: Timo Teras <timo.teras_at_iki.fi>
Date: Mon, 6 Feb 2017 11:43:06 +0200

On Sat, 4 Feb 2017 20:13:54 +0000
Gergo Huszty <huszty.gergo_at_digitaltrip.hu> wrote:

> From: libesz <huszty.gergo_at_digitaltrip.hu>
>
> Nfo parsing related fixes added in a patch.
> - uninitalized string (GetVideoMetadata() - nfo) -> memset to 0
> - stack was kicked with 64k buffer unconditionally (parse_nfo() -
> buf) -> now it is on heap and malloc'd size depends on filesize ---
> community/minidlna/10-minidlna-nfo.patch | 42
> ++++++++++++++++++++++++++++++++
> community/minidlna/APKBUILD | 14 +++++++---- 2 files
> changed, 51 insertions(+), 5 deletions(-) create mode 100644
> community/minidlna/10-minidlna-nfo.patch
>
> diff --git a/community/minidlna/10-minidlna-nfo.patch
> b/community/minidlna/10-minidlna-nfo.patch new file mode 100644
> index 0000000..8e3497c
> --- /dev/null
> +++ b/community/minidlna/10-minidlna-nfo.patch
> _at_@ -0,0 +1,42 @@
> +--- a/metadata.c
> ++++ b/metadata.c
> +_at_@ -160,7 +160,7 @@
> + parse_nfo(const char *path, metadata_t *m)
> + {
> + FILE *nfo;
> +- char buf[65536];
> ++ char *buf;
> + struct NameValueParserData xml;
> + struct stat file;
> + size_t nread;
> +_at_@ -172,11 +172,13 @@
> + DPRINTF(E_INFO, L_METADATA, "Not parsing very
> large .nfo file %s\n", path);
> + return;
> + }
> ++ buf = malloc(file.st_size+1);

The return value should be checked for out-of-memory.

Additionally the code above checks for maximum of 65536, so using
malloc does not help much here. Large files seem to be ignored
intentionally.

Though, if this fixes issues it's probably due to reducing stack usage.
It might be better to increase default stack size, as large stack
buffers might be used elsewhere.

> ++ memset(buf, '\0', file.st_size+1);
> + DPRINTF(E_DEBUG, L_METADATA, "Parsing .nfo file: %s\n",
> path);
> + nfo = fopen(path, "r");
> + if( !nfo )
> + return;
> +- nread = fread(&buf, 1, sizeof(buf), nfo);
> ++ nread = fread(buf, 1, file.st_size, nfo);
> +
> + ParseNameValue(buf, nread, &xml, 0);
> +
> +_at_@ -676,6 +679,7 @@
> +
> + memset(&m, '\0', sizeof(m));
> + memset(&video, '\0', sizeof(video));
> ++ memset(nfo, '\0', sizeof(nfo));
> +
> + //DEBUG DPRINTF(E_DEBUG, L_METADATA, "Parsing video
> %s...\n", name);
> + if ( stat(path, &file) != 0 )

This probable does not fix anything. nfo is strcpy:ied to.

> _at_@ -63,10 +64,13 @@ package() {
>
> md5sums="1970e553a1eb8a3e7e302e2ce292cbc4 minidlna-1.1.5.tar.gz
> 6dd1ec5560ac30d7a04244101e912d45 minidlna.initd
> -59d14c1bf3cd637138bfa58db7255d78 minidlna.confd"
> +59d14c1bf3cd637138bfa58db7255d78 minidlna.confd
> +ddb2a414261109509a81e2ede03e12ba 10-minidlna-nfo.patch"
> sha256sums="8477ad0416bb2af5cd8da6dde6c07ffe1a413492b7fe40a362bc8587be15ab9b

Checksum did not match. There was changes to it after last 'abuild
checksum' ?


---
Unsubscribe:  alpine-aports+unsubscribe_at_lists.alpinelinux.org
Help:         alpine-aports+help_at_lists.alpinelinux.org
---
Received on Mon Feb 06 2017 - 11:43:06 GMT