Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #4081 for 1.2.x #4087

Merged
merged 2 commits into from Feb 9, 2021
Merged

Backport #4081 for 1.2.x #4087

merged 2 commits into from Feb 9, 2021

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Jan 21, 2021

Backported because I need to maintain 1.2.x in Debian 10, and anyone else still stuck on the same branch might as well be able to share them. I've proposed these patches for inclusion in an updated package via security.debian.org.


  • build: Convert environment into a sequence of bwrap arguments

    This means we can systematically pass the environment variables
    through bwrap(1), even if it is setuid and thus is filtering out
    security-sensitive environment variables. bwrap itself ends up being
    run with an empty environment instead.

    This fixes a regression when CVE-2021-21261 was fixed: before the
    CVE fixes, LD_LIBRARY_PATH would have been passed through like this
    and appeared in the flatpak build shell, but during the CVE fixes,
    the special case that protected LD_LIBRARY_PATH was removed in favour
    of the more general flatpak_bwrap_envp_to_args(). That reasoning only
    works if we use flatpak_bwrap_envp_to_args(), consistently, everywhere
    that we run the potentially-setuid bwrap.

    Fixes: 6d1773d "run: Convert all environment variables into bwrap arguments"
    Resolves: flatpak build regression after recent CVE fixes: LD_LIBRARY_PATH not set #4080
    Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=980323
    (cherry picked from commit 9a61d2c)

  • dir: Pass environment via bwrap --setenv when running apply_extra

    This means we can systematically pass the environment variables
    through bwrap(1), even if it is setuid and thus is filtering out
    security-sensitive environment variables. bwrap ends up being
    run with an empty environment instead.

    As with the previous commit, this regressed while fixing CVE-2021-21261.

    Fixes: 6d1773d "run: Convert all environment variables into bwrap arguments"
    (cherry picked from commit fb473ca)

This means we can systematically pass the environment variables
through bwrap(1), even if it is setuid and thus is filtering out
security-sensitive environment variables. bwrap itself ends up being
run with an empty environment instead.

This fixes a regression when CVE-2021-21261 was fixed: before the
CVE fixes, LD_LIBRARY_PATH would have been passed through like this
and appeared in the `flatpak build` shell, but during the CVE fixes,
the special case that protected LD_LIBRARY_PATH was removed in favour
of the more general flatpak_bwrap_envp_to_args(). That reasoning only
works if we use flatpak_bwrap_envp_to_args(), consistently, everywhere
that we run the potentially-setuid bwrap.

Fixes: 6d1773d "run: Convert all environment variables into bwrap arguments"
Resolves: flatpak#4080
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=980323
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 9a61d2c)
This means we can systematically pass the environment variables
through bwrap(1), even if it is setuid and thus is filtering out
security-sensitive environment variables. bwrap ends up being
run with an empty environment instead.

As with the previous commit, this regressed while fixing CVE-2021-21261.

Fixes: 6d1773d "run: Convert all environment variables into bwrap arguments"
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit fb473ca)
@smcv smcv mentioned this pull request Jan 21, 2021
@smcv smcv marked this pull request as ready for review January 21, 2021 18:27
@alexlarsson alexlarsson merged commit 1bfd02c into flatpak:flatpak-1.2.x Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants