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

glibc: cherry-pick fix for CVE-2023-4911 "Looney Tunables" #258856

Closed
wants to merge 1 commit into from
Closed

glibc: cherry-pick fix for CVE-2023-4911 "Looney Tunables" #258856

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 3, 2023

Description of changes

  • Reported by Qualys. Advisory, which notes that:

historically, the processing of environment variables such as LD_PRELOAD, LD_AUDIT, and LD_LIBRARY_PATH has been a fertile source of vulnerabilities in the dynamic loader."

  • There is a working exploit.

  • Upstream fix commit

Things done

  • Built on platform(s)
    • x86_64-linux
    • powerpc64le-linux
    • mips64el-linux
    • aarch64-linux

See also

@ghost ghost marked this pull request as ready for review October 3, 2023 20:11
@ghost ghost added 1.severity: security Issues which raise a security issue, or PRs that fix one backport release-23.05 labels Oct 3, 2023
@ghost ghost changed the title glibc: apply upstream patch for CVE-2023-4911 glibc: cherry-pick fix for CVE-2023-4911 "Looney Tunables" Oct 3, 2023
Copy link
Member

@martinetd martinetd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch matches the relevant part of https://sourceware.org/git/?p=glibc.git;a=commit;h=1056e5b4c3f2d90ed2b4a55f96add28da2f4c8fa "tunables: Terminate if end of input is reached (CVE-2023-4911)"

Thanks!

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Oct 3, 2023
@ofborg ofborg bot added the 10.rebuild-linux-stdenv This PR causes stdenv to rebuild label Oct 3, 2023
@ofborg ofborg bot requested review from edolstra and Ma27 October 3, 2023 22:16
@fiksn
Copy link
Contributor

fiksn commented Oct 3, 2023

Until this gets merged I am running an overlay like this on my machine:

self: super:
let fetchpatch = (import super.path { }).fetchpatch; in
{
  glibc = super.glibc.overrideAttrs (old: rec {
    patches = old.patches ++ [
      (fetchpatch {
        name = "CVE-2023-4911.patch";
        url = "https://sourceware.org/git/?p=glibc.git;a=commitdiff_plain;h=1056e5b4c3f2d90ed2b4a55f96add28da2f4c8fa;hp=0d5f9ea97f1b39f2a855756078771673a68497e1";
        hash = "sha256-3J55jF0I7AUHyh8QAFCDcBCpfymmnlgrlMcSM1/dM6I=";
        excludes = [ "*tst-env-setuid-tunables.c*" ];
      })
    ];
  });
}

this should be equivalent or am I missing something?

@ghost
Copy link
Author

ghost commented Oct 4, 2023

this should be equivalent or am I missing something?

What you wrote will build glibc twice... first it will build an unpatched glibc and use that to build curl and fetchPatch in order to download the fix. Then it will rebuild glibc a second time with the fix.

For personal use, probably not a big deal. Avoiding build-everything-twice is why I had to put a copy of the patch in the PR.

You might be able to avoid this by using fetchurlBoot and the cgit url for the single-file patch (it's nice that cgit offers this!) The URL you want is the one that's in the .nix file in my PR as a comment at the top of the .patch file in this PR. We can't do this in nixpkgs either though, because the patches that cgit spits out aren't guaranteed to be bit-for-bit identical when you upgrade cgit -- things like header formatting and whitespace often change.

PS, you can drop the rec keyword.

@edef1c edef1c mentioned this pull request Oct 4, 2023
12 tasks
@fiksn
Copy link
Contributor

fiksn commented Oct 4, 2023

You might be able to avoid this by using fetchurlBoot and the cgit url for the single-file patch (it's nice that cgit offers this!)

I see, so fetchurlBoot is a way to break this chicken and egg problem.
Actually this one got me quite confused. When I used super.fetchpatch this apparently called the bare-bones boot function which failed with a weird message about:

fetchurl/boot.nix:5:1 called with unexpected argument 'postFetch'

This error message should probably get improved in the lines of "you are using the minimalistic version of fetchurl, make sure to use the real one like this..."

That's why I ended up with:

let fetchpatch = (import super.path { }).fetchpatch; in

which is quite ugly.

We can't do this in nixpkgs either though, because the patches that cgit spits out aren't guaranteed to be bit-for-bit identical when you upgrade cgit -- things like header formatting and whitespace often change.

Oh didn't know that. So if they upgrade cgit build might break because hash will be different. But doesn't fetchpatch already use filterdiff to "normalize" the patch? And hash is then based on that output, meaning any whitespace changes of the cgit output shouldn't matter. But OTOH I can understand that having the patch locally is much easier indeed.

PS, you can drop the rec keyword.

Ah yeah, thx.

@flokli
Copy link
Contributor

flokli commented Oct 4, 2023

Closing this in favor of #258972 (backport PR #258975).

It targets staging, and contains a patchlevel update.

@flokli flokli closed this Oct 4, 2023
@ghost ghost deleted the pr/glibc/cve-2023-4911 branch January 23, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.severity: security Issues which raise a security issue, or PRs that fix one 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants