ldconfig: create /var/cache/ldconfig also with -r
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
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(-)
Comments
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.
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".
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
> 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
would also ignore the setting of _PATH_LDCONFIG_AUX_CACHE.
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?
Thanks!
cheers, josch
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.
@@ -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);
+ }
if (! opt_ignore_aux_cache && aux_cache_file)
load_aux_cache (aux_cache_file);