[pacman-dev] [PATCH 1/2] pacman: fix possible buffer overflow

[Andrew Gregory]

On 09/22/18 at 10:46pm, Morgan Adamiec wrote:
> On Sat, 22 Sep 2018 at 22:20, Andrew Gregory <andrew.gregory.8 at gmail.com> wrote:
> > Good catch, but this approach allows lrealpath to allocate a buffer
> > larger than PATH_MAX only to error if it actually does.  lrealpath is
> > intended to be the same as realpath (aside from not resolving symlinks
> > obviously), so it should be fixed so that it doesn't write more than
> > PATH_MAX into a provided buffer.
> 
> I tried this initially, not really being that familiar with C I
> couldn't really figure it out. How exactly would one communicate the
> error? I can return null but then that could be anything. I could also
> move the alpm_log_error into lrealpath but just seems messy. Maybe
> just truncate and don't error?

Set errno to ENAMETOOLONG and return NULL, just like realpath.

> > We could switch to dynamic allocation in addition to fixing lrealpath,
> > but then the PATH_MAX checks would be pointless and should be removed.
> 
> I assumed PATH_MAX was an OS limit type of thing, so even if we had a
> bigger buffer it would be useless as it would not work properly with
> other functions. I'm guessing that's not the case?

Sort of.  Path limitations are pretty messy, but yes, some functions
will not work correctly with paths longer than PATH_MAX.  We don't
actually use the path after resolving it though; we just do a partial
comparison against paths stored in pacman's database.

> > You also never free the malloc'd path.  You can run the entire test
> > suite under valgrind with `make PY_TEST_FLAGS=--valgrind check` to
> > catch most memory leaks.
> 
> I did free it at some point. Must have accidentally checked it out at
> some point. Whoops