public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Aurelien Jarno <aurelien@aurel32.net>
To: libc-alpha@sourceware.org
Subject: Re: [PATCH] ldconfig: create /var/cache/ldconfig also with -r
Date: Wed, 12 Oct 2022 19:52:21 +0200	[thread overview]
Message-ID: <Y0b+1eVbjzghdQ/G@aurel32.net> (raw)
In-Reply-To: <166554664124.1096741.9694830335625318209@localhost>

[-- Attachment #1: Type: text/plain, Size: 3656 bytes --]

On 2022-10-12 05:50, Johannes Schauer Marin Rodrigues wrote:
> Quoting Aurelien Jarno (2022-10-11 22:59:33)
> > On 2022-10-11 14:20, Johannes Schauer Marin Rodrigues wrote:
> > > Without the -r option, ldconfig creates /var/cache/ldconfig if it didn't
> > > exist yet. With the -r option, a non-existing /var/cache/ldconfig inside
> > > the chroot directory will *not* get created because chroot_canon() will
> > > return NULL if the path doesn't exist. This means that aux_cache_file
> > > will be set to NULL and save_aux_cache() doesn't get executed at the
> > > end. So instead of using chroot_canon() to prepending the chroot path,
> > > combine the paths manually.
> > > ---
> > >  elf/ldconfig.c | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/elf/ldconfig.c b/elf/ldconfig.c
> > > index e6c24e71a4..da76dc31b8 100644
> > > --- a/elf/ldconfig.c
> > > +++ b/elf/ldconfig.c
> > > @@ -1293,9 +1293,11 @@ main (int argc, char **argv)
> > >       add_system_dir (LIBDIR);
> > >      }
> > >  
> > > -  const char *aux_cache_file = _PATH_LDCONFIG_AUX_CACHE;
> > > -  if (opt_chroot != NULL)
> > > -    aux_cache_file = chroot_canon (opt_chroot, aux_cache_file);
> > > +  char *aux_cache_file = (char *)(_PATH_LDCONFIG_AUX_CACHE);
> > > +  if (opt_chroot != NULL) {
> > > +    aux_cache_file = alloca (strlen (opt_chroot) + strlen (_PATH_LDCONFIG_AUX_CACHE) + 2);
> > > +    sprintf (aux_cache_file, "%s/%s", opt_chroot, _PATH_LDCONFIG_AUX_CACHE);
> > > +  }
> > 
> > This drops the use chroot_canon() call. I am afraid it might allows one
> > to "escape" the "chroot". Imagine that /var/cache/ldconfig is a symlink
> > pointing outside of the opt_chroot.
> 
> This code path (opt_chroot being NULL) is only reached if -r was specified and
> the chroot() call didn't succeed. This happens, for example, when the user uses
> fakeroot. So one can argue that:
> 
>  - escaping the "chroot" is not harmful because there were no sufficient
>    permissions to chroot() in the first place
>  - the user is running ldconfig in a non-default environment where I think they
>    might be expected to keep the pieces

I agree that the "security" implication are quite low with this option,
but that is still a change from the existing behaviour. We should have
the opinion of others to know if this is acceptable.

> > To avoid changing the code too much, one way could be to call
> > chroot_canon(opt_chroot, "/var/cache/ldconfig") and concatenate the result
> > with "aux-cache".
> 
> Calling chroot_canon(opt_chroot, "/var/cache/ldconfig") will still return NULL
> because /var/cache/ldconfig might not exist inside the -r directory. Doing so

chroot_canon() allows the last element of the path to not exist. That is
actually why it works when calling it with _PATH_LDCONFIG_AUX_CACHE
where the aux-cache file does not exist.

> would also ignore the setting of _PATH_LDCONFIG_AUX_CACHE.

Yep, what I really meant was dirname(_PATH_LDCONFIG_AUX_CACHE), but i
used the explicit path to avoid confusion.

> Maybe what would be done to prevent the "escaping" was to first call the
> original chroot_canon(opt_chroot, aux_cache_file) and, if the result is NULL,
> concatenate the paths manually instead?

That should limit the risk, but not totally. If /var/cache/ldconfig is a
symlink pointing outside the chroot, chroot_canon() will return NULL.
Concatenating the two paths will result in a path outside of the chroot.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2022-10-12 17:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-11 12:20 Johannes Schauer Marin Rodrigues
2022-10-11 20:59 ` Aurelien Jarno
2022-10-12  3:50   ` Johannes Schauer Marin Rodrigues
2022-10-12 17:52     ` Aurelien Jarno [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y0b+1eVbjzghdQ/G@aurel32.net \
    --to=aurelien@aurel32.net \
    --cc=libc-alpha@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).