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

(RHEL-18776) CVE-2022-4415 systemd: local information leak due to systemd-coredump not respecting fs.suid_dumpable kernel setting #423

Merged
merged 3 commits into from
Jan 21, 2024

Conversation

brozs
Copy link
Collaborator

@brozs brozs commented Jan 17, 2024

I have cherry-picked three existing commits from the rhel-8.7.0 branch:

Related prerequisities for 9fa7f77:
84a8245
e256e03

And the fix itself:
9fa7f77

Resolves: RHEL-18776

(cherry picked from commit 2c5d05b3cd986568105d67891e4010b868dea24f)

Related: RHEL-18776
@github-actions github-actions bot added tracker/missing Formerly needs-bz pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Jan 17, 2024
Copy link

github-actions bot commented Jan 17, 2024

Commit validation

Tracker - RHEL-18776

The following commits meet all requirements

commit upstream
5b3becc - basic: add STRERROR() wrapper for strerror_r() systemd/systemd@2c5d05b
f8c8043 - coredump: put context array into a struct systemd/systemd@f46c706
feb99a5 - coredump: do not allow user to access coredumps with changed uid/gid/c… systemd/systemd@3e4d0f6

Tracker validation

Success

🟢 Tracker RHEL-18776 has set desired product: rhel-8.6.0.z
🟢 Tracker RHEL-18776 has set desired component: systemd
🟢 Tracker RHEL-18776 has been approved


Pull Request validation

Success

🟢 CI - All checks have passed
🟢 Review - Reviewed by a member
🟢 Approval - Changes were approved


Auto Merge

Failed

🔴 Pull Request has unsupported target branch rhel-8.6.0, expected branches are: 'main,master'

Success

🟢 Pull Request is not marked as draft and it's not blocked by dont-merge label
🟢 Pull Request meet requirements, title has correct form
🟢 Pull Request meet requirements, mergeable is true
🟢 Pull Request meet requirements, mergeable_state is clean

Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

Please drop # from Resolves in commit message of 7692d85

Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

Please use plain text and full sha in cherry-pick - 551a992

dtardon and others added 2 commits January 17, 2024 16:39
[dtardon: This is based on commit f46c706 ,
but does just the minimal change to introduce the Context struct that is
needed by the following commit.]

This has been cherry picked from a downstream commit
e256e03

(cherry picked from commit f46c706)

Related: RHEL-18776
…capabilities

When the user starts a program which elevates its permissions via setuid,
setgid, or capabilities set on the file, it may access additional information
which would then be visible in the coredump. We shouldn't make the the coredump
visible to the user in such cases.

Reported-by: Matthias Gerstner <[email protected]>

This reads the /proc/<pid>/auxv file and attaches it to the process metadata as
PROC_AUXV. Before the coredump is submitted, it is parsed and if either
at_secure was set (which the kernel will do for processes that are setuid,
setgid, or setcap), or if the effective uid/gid don't match uid/gid, the file
is not made accessible to the user. If we can't access this data, we assume the
file should not be made accessible either. In principle we could also access
the auxv data from a note in the core file, but that is much more complex and
it seems better to use the stand-alone file that is provided by the kernel.

Attaching auxv is both convient for this patch (because this way it's passed
between the stages along with other fields), but I think it makes sense to save
it in general.

We use the information early in the core file to figure out if the program was
32-bit or 64-bit and its endianness. This way we don't need heuristics to guess
whether the format of the auxv structure. This test might reject some cases on
fringe architecutes. But the impact would be limited: we just won't grant the
user permissions to view the coredump file. If people report that we're missing
some cases, we can always enhance this to support more architectures.

I tested auxv parsing on amd64, 32-bit program on amd64, arm64, arm32, and
ppc64el, but not the whole coredump handling.

(cherry picked from commit 3e4d0f6cf99f8677edd6a237382a65bfe758de03)

Resolves: RHEL-18776
Copy link
Member

@msekletar msekletar left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added pr/needs-manual-merge and removed pr/needs-review Formerly needs-review labels Jan 20, 2024
@jamacku jamacku merged commit 7134cf2 into redhat-plumbers:rhel-8.6.0 Jan 21, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants