~alpine/aports

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

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

Gergo Huszty <huszty.gergo@digitaltrip.hu>
Details
Message ID
<20170204201354.26019-1-huszty.gergo@digitaltrip.hu>
Sender timestamp
1486239234
DKIM signature
missing
Download raw message
7 years ago
Patch: +51 -5 
From: libesz <huszty.gergo@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
@@ -0,0 +1,42 @@
--- a/metadata.c
+++ b/metadata.c
@@ -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;
@@ -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);
+	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);
 
@@ -230,6 +232,7 @@
 
 	ClearNameValueList(&xml);
 	fclose(nfo);
+	free(buf);
 }
 
 void
@@ -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 )
diff --git a/community/minidlna/APKBUILD b/community/minidlna/APKBUILD
index cab6b58..d66ffdb 100644
--- a/community/minidlna/APKBUILD
+++ b/community/minidlna/APKBUILD
@@ -2,12 +2,13 @@
# Maintainer: Francesco Colista <francesco.colista@gmail.com>
pkgname=minidlna
pkgver=1.1.5
pkgrel=3
pkgrel=4
pkgdesc="A small dlna server"
url="http://sourceforge.net/projects/minidlna/"
arch="all"
license="GPL"
depends=
options=
makedepends="
	bsd-compat-headers
	libvorbis-dev
@@ -26,7 +27,7 @@ pkggroups="$pkgname"
source="http://downloads.sourceforge.net/project/minidlna/minidlna/$pkgver/minidlna-$pkgver.tar.gz
	$pkgname.initd
	$pkgname.confd
	"
	10-minidlna-nfo.patch"

builddir="$srcdir/$pkgname-$pkgver"

@@ -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  minidlna-1.1.5.tar.gz
251e790bb8adb91b4d00dd47543b9b02409879ade0853ebc3bd0fc0184bd485e  minidlna.initd
67603d65c6bd3918255f050cb5cfd6fc1373b024bca1ce728f03491a90d79e19  minidlna.confd"
67603d65c6bd3918255f050cb5cfd6fc1373b024bca1ce728f03491a90d79e19  minidlna.confd
26311a96a6a4a916c150839e46e5c4a2422fe00f1629304e6fb5d7dddacdc84e  10-minidlna-nfo.patch"
sha512sums="2a8eaa42fcda6f98648f1726af5cdba6d2358c386440dd0de933364cfbd1ced2fee5f883033e1a5a692b760749beb2c12798020a3591ddcea22663102d4f3dfa  minidlna-1.1.5.tar.gz
e16961bb68c004297f1e26422b1d15bd8583ba2e0e36c88902a45573b685993fff88d2d0dae8c624eaeddb0deca614dbc13b8345f34b4c348961c00b05c0df30  minidlna.initd
e209848af0d79069ac989ad61d3be610b4c0c2783a207a50463a25ec3811b04d1da3a2acde54749878bec44e1567874ede827b978d5472c00f6a855663e5cbf8  minidlna.confd"
e209848af0d79069ac989ad61d3be610b4c0c2783a207a50463a25ec3811b04d1da3a2acde54749878bec44e1567874ede827b978d5472c00f6a855663e5cbf8  minidlna.confd
4be55ca6c4eeabcff7406088fb2cd4623dd8aebc45d81905cb4f1d24b3933a75c07af896ce55aeeb9dfe2598781f1b9e8cf9aff67cbd6fbcf763181642f9b77f  10-minidlna-nfo.patch"
-- 
2.8.3



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---

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

Huszty Gergő <huszty.gergo@digitaltrip.hu>
Details
Message ID
<f295c0904ef231d113638c948c9374f2@digitaltrip.hu>
In-Reply-To
<20170204201354.26019-1-huszty.gergo@digitaltrip.hu> (view parent)
Sender timestamp
1486240032
DKIM signature
missing
Download raw message
7 years ago 
Hi, 

 Forgot to mention that it is also reported to the minidlna project:
https://sourceforge.net/p/minidlna/bugs/294/ 

Br,
 Gergo 

2017-02-04 21:13 időpontban Gergo Huszty ezt írta:

> From: libesz <huszty.gergo@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
> @@ -0,0 +1,42 @@
> +--- a/metadata.c
> ++++ b/metadata.c
> +@@ -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;
> +@@ -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);
> ++    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);
> + 
> +@@ -230,6 +232,7 @@
> + 
> +     ClearNameValueList(&xml);
> +     fclose(nfo);
> ++    free(buf);
> + }
> + 
> + void
> +@@ -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 )
> diff --git a/community/minidlna/APKBUILD b/community/minidlna/APKBUILD
> index cab6b58..d66ffdb 100644
> --- a/community/minidlna/APKBUILD
> +++ b/community/minidlna/APKBUILD
> @@ -2,12 +2,13 @@
> # Maintainer: Francesco Colista <francesco.colista@gmail.com>
> pkgname=minidlna
> pkgver=1.1.5
> -pkgrel=3
> +pkgrel=4
> pkgdesc="A small dlna server"
> url="http://sourceforge.net/projects/minidlna/"
> arch="all"
> license="GPL"
> depends=
> +options=
> makedepends="
> bsd-compat-headers
> libvorbis-dev
> @@ -26,7 +27,7 @@ pkggroups="$pkgname"
> source="http://downloads.sourceforge.net/project/minidlna/minidlna/$pkgver/minidlna-$pkgver.tar.gz
> $pkgname.initd
> $pkgname.confd
> -    "
> +    10-minidlna-nfo.patch"
> 
> builddir="$srcdir/$pkgname-$pkgver"
> 
> @@ -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  minidlna-1.1.5.tar.gz
> 251e790bb8adb91b4d00dd47543b9b02409879ade0853ebc3bd0fc0184bd485e  minidlna.initd
> -67603d65c6bd3918255f050cb5cfd6fc1373b024bca1ce728f03491a90d79e19  minidlna.confd"
> +67603d65c6bd3918255f050cb5cfd6fc1373b024bca1ce728f03491a90d79e19  minidlna.confd
> +26311a96a6a4a916c150839e46e5c4a2422fe00f1629304e6fb5d7dddacdc84e  10-minidlna-nfo.patch"
> sha512sums="2a8eaa42fcda6f98648f1726af5cdba6d2358c386440dd0de933364cfbd1ced2fee5f883033e1a5a692b760749beb2c12798020a3591ddcea22663102d4f3dfa  minidlna-1.1.5.tar.gz
> e16961bb68c004297f1e26422b1d15bd8583ba2e0e36c88902a45573b685993fff88d2d0dae8c624eaeddb0deca614dbc13b8345f34b4c348961c00b05c0df30  minidlna.initd
> -e209848af0d79069ac989ad61d3be610b4c0c2783a207a50463a25ec3811b04d1da3a2acde54749878bec44e1567874ede827b978d5472c00f6a855663e5cbf8  minidlna.confd"
> +e209848af0d79069ac989ad61d3be610b4c0c2783a207a50463a25ec3811b04d1da3a2acde54749878bec44e1567874ede827b978d5472c00f6a855663e5cbf8  minidlna.confd
> +4be55ca6c4eeabcff7406088fb2cd4623dd8aebc45d81905cb4f1d24b3933a75c07af896ce55aeeb9dfe2598781f1b9e8cf9aff67cbd6fbcf763181642f9b77f  10-minidlna-nfo.patch"
Timo Teras <timo.teras@iki.fi>
Details
Message ID
<20170206114306.4920c40c@vostro.util.wtbts.net>
In-Reply-To
<20170204201354.26019-1-huszty.gergo@digitaltrip.hu> (view parent)
Sender timestamp
1486374186
DKIM signature
missing
Download raw message
7 years ago 
On Sat,  4 Feb 2017 20:13:54 +0000
Gergo Huszty <huszty.gergo@digitaltrip.hu> wrote:

> From: libesz <huszty.gergo@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
> @@ -0,0 +1,42 @@
> +--- a/metadata.c
> ++++ b/metadata.c
> +@@ -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;
> +@@ -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);
> + 
> +@@ -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.

> @@ -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@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---
Reply to thread Export thread (mbox)