[pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

2016-06-02 Thread Tobias Stoeckmann
It is possible (but unlikely) to encounter very long symbolic links, which would take a few mega bytes in size. In general, such long symbolic links are impossible to create because the kernel will only accept up to PATH_MAX in length. BUT lstat() on a maliciously modified symbolic link can return

Re: [pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

2016-06-04 Thread Tobias Stoeckmann
h style - also properly format if() instead of if () >From 1c075c6b70c21c22636fa2ab2e9ef17bfcb4aaa5 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sat, 4 Jun 2016 12:44:43 +0200 Subject: [PATCH] Prevent stack overflow on symbolic link access. It is possible (but unlikely) to encounter very lon

[pacman-dev] [PATCH] Always use proper error code in alpm_initialize.

2016-06-05 Thread Tobias Stoeckmann
In out of memory conditions, an undefined error value is written into *err, because myerr is never explicitly set in these cases. I have also converted a calloc into a MALLOC call, because the memory will be properly filled by the snprintf call right after it. Signed-off-by: Tobias Stoeckmann

[pacman-dev] [PATCH] Reject files larger than INT_MAX in read_sigfile.

2016-06-05 Thread Tobias Stoeckmann
race condition between stat() and fopen(). Signed-off-by: Tobias Stoeckmann --- lib/libalpm/be_package.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index c9ed770..055fb1e 100644 --- a/lib/libalpm/be_package.c ++

Re: [pacman-dev] [PATCH] Reject files larger than INT_MAX in read_sigfile.

2016-06-05 Thread Tobias Stoeckmann
race condition between stat() and fopen(). Signed-off-by: Tobias Stoeckmann --- Thanks for pointing out the flaw Florian! --- lib/libalpm/be_package.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index c9ed770.

[pacman-dev] [PATCH] Release resources on error paths.

2016-06-05 Thread Tobias Stoeckmann
Some resources (memory or file descriptors) are not released on all error paths. Signed-off-by: Tobias Stoeckmann --- Yes it's rather ironic to send this patch after forgetting one on my own just now. ;) --- lib/libalpm/add.c| 5 - lib/libalpm/backup.c | 5 +++-- lib/li

Re: [pacman-dev] [PATCH] Reject files larger than INT_MAX in read_sigfile.

2016-06-06 Thread Tobias Stoeckmann
race condition between stat() and fopen(). Signed-off-by: Tobias Stoeckmann --- I don't know about any sane limitation of signature files, so I just took INT_MAX. It's an implementation limit of pacman. --- lib/libalpm/be_package.c | 11 ++- 1 file changed, 6 insertions(+), 5 delet

Re: [pacman-dev] [PATCH] Release resources on error paths.

2016-06-06 Thread Tobias Stoeckmann
Some resources (memory or file descriptors) are not released on all error paths. Signed-off-by: Tobias Stoeckmann --- Thanks for your valueable review, Andrew. :) --- lib/libalpm/add.c| 5 - lib/libalpm/backup.c | 5 +++-- lib/libalpm/be_local.c | 19 --- lib

Re: [pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

2016-06-06 Thread Tobias Stoeckmann
On Mon, Jun 06, 2016 at 01:27:52AM -0400, Andrew Gregory wrote: > Since we're already talking about unlikely scenarios... My reading of > readlink(2) and readlink(3p) suggest this might still run into > problems on oddly configured systems. POSIX leaves up to the > implementation what happens if

Re: [pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

2016-06-06 Thread Tobias Stoeckmann
gnulib, buggy file systems can report garbage in st_size, so there is no benefit in checking length against it. Signed-off-by: Tobias Stoeckmann --- src/pacman/check.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/pacman/check.c b/src/pacman/check.c

[pacman-dev] [PATCH] Fix OOB read and endless loop in signature parser.

2016-06-06 Thread Tobias Stoeckmann
his patch does not use GCC-specific overflow checking routines, because I am not sure if that's applicable for pacman's design goals. Signed-off-by: Tobias Stoeckmann --- Feedback is very appreciated. :) --- lib/libalpm/signing.c | 46 -- 1

Re: [pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

2016-06-09 Thread Tobias Stoeckmann
On Thu, Jun 09, 2016 at 04:25:36PM -0400, Andrew Gregory wrote: > SIZE_MAX is the maximum size of individual objects, not the entire > addressable space. I hope you know that it becomes very very theoretical now, and that there is no such system out there... Let's assume that there is a system in

Re: [pacman-dev] [PATCH] Prevent stack overflow on symbolic link access.

2016-06-09 Thread Tobias Stoeckmann
gnulib, buggy file systems can report garbage in st_size, so there is no benefit in checking length against it. Signed-off-by: Tobias Stoeckmann --- src/pacman/check.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/pacman/check.c b/src/pacman/check.c

[pacman-dev] [PATCH] Fix out of boundary reads in pacsort.

2016-06-09 Thread Tobias Stoeckmann
arithmetic on it turns it into SIZE_MAX. Eventually, pacman segfaults while traversing this memory area for needed chars: $ echo "-/.pkg.tar.xz" | pacsort Segmentation fault Signed-off-by: Tobias Stoeckmann --- src/util/pacsort.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-

[pacman-dev] [PATCH] mbscasecmp fails in 1% of time.

2016-06-10 Thread Tobias Stoeckmann
exists that could trigger this. Signed-off-by: Tobias Stoeckmann --- There is a reason that I picked this one, annotation will help. --- src/pacman/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 81780f7..e38ef06 100644 --- a

Re: [pacman-dev] [PATCH] Fix out of boundary reads in pacsort.

2016-06-10 Thread Tobias Stoeckmann
On Thu, Jun 09, 2016 at 07:46:00PM -0400, Andrew Gregory wrote: > -t and -k are used to sort tabular data, where NUL is a perfectly sane > field separator. In fact, with --machinereadable pacman will output > a table of NUL separated fields. Though, pacsort does need its > handling of -t '\0' fix

Re: [pacman-dev] [PATCH] mbscasecmp fails in 1% of time.

2016-06-11 Thread Tobias Stoeckmann
On Fri, Jun 10, 2016 at 07:10:56PM -0400, Andrew Gregory wrote: > Good catch, but the solution needs work. You were able to grasp the idea of this non-issue. Now you can adjust your code the way you want.

Re: [pacman-dev] [PATCH] Fix out of boundary reads in pacsort.

2016-06-11 Thread Tobias Stoeckmann
On Fri, Jun 10, 2016 at 07:11:16PM -0400, Andrew Gregory wrote: > I was saying that your rationale for just dropping support for -t '\0' > was incorrect. NUL not being legal in a path name is irrelevant. Yup, it's irrelevant. The relevant part is using strndup() with an input line. Maybe you want

Re: [pacman-dev] [PATCH] Reject files larger than INT_MAX in read_sigfile.

2016-06-18 Thread Tobias Stoeckmann
between stat() and fopen(). Signed-off-by: Tobias Stoeckmann --- lib/libalpm/be_package.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index c9ed770..8307d81 100644 --- a/lib/libalpm/be_package.c +++ b/lib

Re: [pacman-dev] [PATCH] Fix out of boundary reads in pacsort.

2016-06-18 Thread Tobias Stoeckmann
On Mon, Jun 13, 2016 at 04:01:17PM +1000, Allan McRae wrote: > Fixing that strndup is preferable. I want to keep the ability to handle > \0 delimited fields given that is what pacman --machinereadable does > (despite that option not being ubiquitous at the moment...) If \0 is a valid field _and_

Re: [pacman-dev] [PATCH] Handle all POSIX compliant systems in mbscasecmp.

2016-06-18 Thread Tobias Stoeckmann
: Tobias Stoeckmann --- src/pacman/util.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 81780f7..b979083 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1503,6 +1503,8 @@ int select_question(int count) return