[pacman-dev] Missing sanity checks for path -> weird behaviors and segfaults

[Dan McGee]

On 7/22/07, Xavier <shiningxc at gmail.com> wrote:
> If Root isn't specified, paths like (null)var/ will be created.
> I think pacman should either default to / for Root, or just fail if it isn't
> set.
> Also, if no Cache Dirs is set, pacman will try to fallback to /tmp/ , but
> then it segfaults. (try pacman -Sw bar).
>
> Apparently it segfaults on this in libalpm/sync.c , sync_commit function,
> after downloading the file succesfully :
> 718   for(i = handle->dbs_sync; i; i = i->next)
>
> I found it weird that inside that loop, i was used a second time in an inner
> loop :
> 758       for(i = alpm_option_get_cachedirs(); i; i = alpm_list_next(i)) {
>
> I thought that was the problem, but it didn't fix the segfault.
>
> Second strange thing is that the files list seem to be freed in any cases :
>  795       FREELIST(files);
>
> but it's used at the end of that _alpm_sync_commit function :
> 1018   if(!validcache && !(trans->flags & PM_TRANS_FLAG_DOWNLOADONLY)) {
> 1019     /* delete packages */
> 1020     for(i = files; i; i = i->next) {
> 1021       unlink(i->data);
> 1022     }
> 1023   }
>
> Finally, if the cache isn't valid (so it fallbacks to /tmp/) and DOWNLOADONLY is used,
> files won't be deleted, but will they be used next times?
> Because the fallback (lines 779-790) happen after the check of existence of
> the package in the cache (lines 740-744).
> I can't check what happens because of the segfault.

This is something I've been putting off but definitely needs doing. We
need to do some checks both in the front end and back end for valid
path specifications.

I think hardcoding a default root of "/" is fine (or using the one
specified at ./configure time)? Any thoughts here appreciated. I think
it would be better to use those specified by configure, so I'm going
to readd some of that stuff to the pacman front end this week
otherwise the upgrade path will be a bit rough for most people.

-Dan