Skip to content

Commit 86e7a33

Browse files
committed
sosreport: Fix command injection with crafted report names [CVE-2024-2947]
Files in /var/tmp/ are controllable by any user. In particular, an unprivileged user could create an sosreport* file containing a `'` and a shell command, which would then run with root privileges when the admin Cockpit user tried to delete the report. Use the `cockpit.file()` API instead, which entirely avoids shell. The main motivation for using shell and the glob was to ensure that the auxiliary files like *.gpg and *.sha256 get cleaned up -- do that explicitly (which is much safer anyway), and let our tests make sure that we don't leave files behind. https://bugzilla.redhat.com/show_bug.cgi?id=2271614 https://bugzilla.redhat.com/show_bug.cgi?id=2271815 Cherry-picked from main commit 9c4cc9b
1 parent 76433fc commit 86e7a33

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

pkg/sosreport/index.jsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,16 @@ function sosDownload(path) {
218218
}
219219

220220
function sosRemove(path) {
221-
return cockpit.script(cockpit.format("rm -f '$0' '$0'.*", path), { superuser: true, err: "message" });
221+
// there are various potential extra files; not all of them are expected to exist,
222+
// the file API tolerates removing nonexisting files
223+
const paths = [
224+
path,
225+
path + ".asc",
226+
path + ".gpg",
227+
path + ".md5",
228+
path + ".sha256",
229+
];
230+
return Promise.all(paths.map(p => cockpit.file(p, { superuser: true }).replace(null)));
222231
}
223232

224233
const SOSDialog = () => {

test/verify/check-sosreport

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ only-plugins=release,date,host,cgroups,networking
8989
# while the download is ongoing, it will have an *.xz.tmpsuffix name, gets renamed to *.xz when done
9090
wait(lambda: len(downloaded_sosreports()) > 0)
9191
report_gpg = downloaded_sosreports()[0]
92+
report = report_gpg.removesuffix(".gpg")
9293
base_report_gpg = os.path.basename(report_gpg)
93-
report = report_gpg.replace(".gpg", "")
94+
base_report = base_report_gpg.removesuffix(".gpg")
9495

9596
m.execute(f"test -f /var/tmp/{base_report_gpg}")
9697

@@ -113,7 +114,8 @@ only-plugins=release,date,host,cgroups,networking
113114
b.click("tr:contains(mylabel) button.pf-c-dropdown__toggle")
114115
b.click("tr:contains(mylabel) li:contains(Delete)")
115116
b.click("#sos-remove-dialog button:contains(Delete)")
116-
wait(lambda: m.execute(f"! test -f /var/tmp/{base_report_gpg} && echo yes"))
117+
# ensure it removes the report itself, and auxiliary files like .gpg
118+
m.execute(f"while ls /var/tmp/{base_report}*; do sleep 1; done", stdout=None, timeout=10)
117119

118120
self.allow_journal_messages('.*comm="sosreport".*')
119121

0 commit comments

Comments
 (0)