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
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
@@ -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
@@ -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
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"
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
---