CVE-2024-3955: command injection in CraftBeerPi 4

The target#

CraftBeerPi is a brewing controller. You install it on a Raspberry Pi, wire temperature probes and relays to the GPIO header, and it runs your mash schedule: reading sensors, switching heating elements, driving pumps, holding a step at 67 °C for ninety minutes while you do something else. It is a friendly, well-liked piece of hobbyist software, and version 4 is a Python application built on aiohttp with a web UI you open from your phone in the garage.

That combination — a small HTTP server, a Raspberry Pi, and physical actuators capable of boiling fifty litres of liquid — is what makes the bug worth writing up. The vulnerability itself is about as classic as they come.

The endpoint#

The system endpoints are registered under a /system prefix. One of them zips up the service journal and streams it back, so you can attach logs to a bug report. It takes a single parameter: how many hours of history to include.

161@request_mapping("/log/{logtime}/", method="GET", name="BackupConfig", auth_required=False)
162async def downloadlog(self, request):
163    """ ... zip and download craftbeerpi.service log ... """
174    logtime = request.match_info['logtime']
175    await self.controller.downloadlog(logtime)
176    filename = "cbpi4_log.zip"

Three things are worth noting before we go further. The parameter arrives through match_info, meaning it is a path segment rather than a query string — the advisory calls it a “URL GET parameter”, which is true of the request method but slightly imprecise about where the value sits. It is passed onward without so much as a length check. And the handler is declared auth_required=False, which we will come back to, because it means less than it appears to.

The sink#

The controller does the actual work, and it does it by shelling out to journalctl.

67async def downloadlog(self, logtime):
68    filename = "cbpi4.log"
69    fullname = pathlib.Path(os.path.join(".",filename))
     # ... other output paths elided ...
81    if logtime == "b":
82        os.system('journalctl -b -u craftbeerpi.service --output cat > {}'.format(fullname))
83    else:
84        os.system('journalctl --since \"{} hours ago\" -u craftbeerpi.service --output cat > {}'.format(logtime, fullname))

Line 84 is the whole vulnerability. A caller-controlled string is interpolated into a command with str.format and handed to os.system, which runs it through /bin/sh.

The branch above it is interesting for the opposite reason. Line 82 builds a command the same careless way, but the only value it interpolates is a constant computed inside the function, and the branch is guarded by a literal comparison against "b". It is safe entirely by accident of structure. That accident turns out to matter later.

Why the quotes do not help#

The reflex on seeing this is that the value is quoted, so an attacker would first have to escape the quotes — and that a filter on " and ; would therefore be enough. It would not. The shell performs command substitution inside double quotes. Both $(…) and backticks are expanded there, so nothing has to be broken out of at all.

There is one genuine constraint, and it comes from the router rather than the shell. aiohttp’s dynamic path segments match everything except a slash, so / cannot appear in the payload. Absolute paths and most download-and-run one-liners are therefore off the table in a single request. Spaces are fine once percent-encoded, and $, (, ) and backticks all pass through untouched. It is a speed bump, not a boundary: ${IFS} covers whitespace, and anything that really needs a slash can be staged in a second request.

Proof of concept#

The neatest demonstration proves execution and privilege in the same request: run id and redirect its output to a file in the working directory. No slash required, nothing destroyed, and the answer is sitting in /home/pi afterwards.

What the shell actually receives is this, with the substitution running before journalctl is ever invoked:

The request returns an error, because journalctl is unimpressed by an empty --since and the handler then fails to open the zip it expected. The side effect has already happened by then. Blind command execution is still command execution.

Impact#

The privilege claim is worth substantiating rather than assuming, because “runs as root” is asserted casually far too often. The service unit shipped in the repository is four lines long and declares no User=. systemd defaults to root in that case.

The same file gives independent corroboration. Elsewhere in system_controller.py there are calls to os.system('systemctl reboot') and os.system('systemctl poweroff'), with no sudo anywhere. Those only work for a privileged process. The software expects to be root, and the unit file makes sure it is.

A word about authentication#

It would be easy to write this up as a pre-authentication remote code execution and imply that someone forgot a decorator on one sensitive endpoint. That would misrepresent the codebase. In the affected version, auth_required=True appears exactly once across every HTTP module — on /logout. There is a login endpoint backed by aiohttp_auth, with credentials defaulting to cbpi / cbpi, but nothing else in the application is behind it. Actors, kettles, sensors, configuration, file upload, reboot, shutdown — all open.

So this is not an authentication bypass. It is an appliance designed on the assumption that the network is the boundary, which is a defensible choice for something that lives on a workshop LAN. The point is what that assumption costs when a command injection turns up: there is no second line to fall back on. The absence of authentication does not create the bug, it removes every obstacle between the bug and the attacker.

It is a GET request#

This is the part that lifts the finding above “an attacker on your LAN can own your Pi”. The endpoint is a GET, it needs no authentication, and it carries no CSRF token. Any web page loaded by any browser on that network can therefore reach it — an <img> pointing at the controller’s address is enough. The response is opaque to the attacker’s page, and irrelevant, because the command has already run.

That converts a local-network issue into a drive-by: the brewer reads an article, and something on the page fires a request at a well-known hostname on a well-known port. Modern browsers have since tightened this considerably — Chrome’s Private Network Access checks are aimed squarely at public pages reaching private addresses — but coverage was patchy in 2024 and is still not uniform across browsers. It is a real avenue, and it is why an unauthenticated GET that executes shell commands deserves the score it got.

The patch#

The maintainer fixed it the day after the report. Commit 57572c7, 15 April 2024, message: “add check for logtime parameter in http_system endpoint as first test to address issue #132”. Same commit bumped the version from 4.4.0 to 4.4.1.a1, which is the boundary recorded in the advisory.

As a filter, this holds up. It is an allowlist rather than a denylist: the value must survive int() or equal "b". Every string Python’s int() accepts consists of optional surrounding whitespace, an optional sign, digits, and underscores as separators. Not one of those is a shell metacharacter. I could not find a value that passes the check and still reaches the shell as anything but an inert number.

The right check at the wrong layer#

What the patch does not do is touch line 84. After the fix, SystemController.downloadlog() still formats its argument into a shell string and still runs it through os.system. It remains a general-purpose command execution primitive that happens to be safe because its single caller happens to validate first.

That is a fragile arrangement, and the fragility is not hypothetical for this project specifically: CraftBeerPi has a plugin system. A second caller — a plugin, a CLI path, an MQTT handler, a future endpoint written by someone who reasonably assumes a controller method validates its own inputs — reintroduces the vulnerability without touching the line that contains it. Defences that live one layer above the sink protect the paths you thought of.

The real fix, three weeks later#

To the maintainer’s considerable credit, the story does not end at the input filter. On 1 and 2 May 2024 there is a run of commits rewriting log export altogether. 858c718 (“simplify log download”) replaces the shell invocations with the systemd journal Python API, and f5593fd (“remove old downloadlog function”) deletes the leftover copy that still carried both os.system('journalctl …') calls.

This is the shape a fix should have. There is no shell, so there is nothing to inject into; the conversion to int now happens at the point of use, where it is a type requirement rather than a security control; and a hostile value fails with a ValueError instead of a subprocess. The dangerous primitive is gone rather than guarded, which means no future caller can misuse it.

Disclosure#

1. 2024-04-14

   Reported

   Issue #132 opened against PiBrewing/craftbeerpi4, pointing at the sink and naming the affected range from 4.0.0.58.
2. 2024-04-15

   Patched — one day later

   Commit 57572c7 adds the parameter check and releases 4.4.1.a1. A turnaround most vendors with a security team do not manage.
3. 2024-04-17

   CERT.PL takes coordination

   Report submitted to the national CVD team, who verified the fix and assessed it for a CVE.
4. 2024-04-18

   CVE-2024-3955 reserved

   Draft advisory circulated for review. The draft transposed the two file paths — the same transposition I had made in the original issue — and it was corrected before publication.
5. 2024-05-02

   Advisory published

   CERT.PL publishes the advisory, credited to Pondzik. CWE-94, CVSS 3.1 9.8. On the same day, upstream removes the shell call entirely.
6. TODAY

   Resolved upstream

   Current releases contain no shell invocation on this path. Users on anything older than 4.4.1.a1 should still upgrade.

Takeaways#

The grep that finds this class of bug is not sophisticated. Look for os.system, subprocess with shell=True, or popen, on the same line as .format(), an f-string, or %. Then work backwards to see whether anything reaching it comes from a request. In a project of this size that is an afternoon.

Three things stand out to me looking back at it.

Quoting is not validation. The value in line 84 was quoted, and it made no difference, because the shell expands substitutions inside double quotes. Any reasoning of the form “it is inside quotes, so it is contained” needs to be checked against what the shell actually does rather than what the quotes look like they do.

Validate at the sink, not just at the door. The initial patch was correct and shipped in a day, and I would take that trade every time over a slow perfect fix. But it left a shell-injection primitive sitting in a controller method, safe only for as long as nobody added a second caller. Removing the primitive is what actually closed the issue.

And an unauthenticated GET is worse than it looks. Because a GET can be triggered cross-origin without a token, the reachable population is not “attackers on the LAN” but “anything the user’s browser loads”. Where a design decides the network is the security boundary, side-effecting GET endpoints are the place that decision leaks.

References#

1. CERT.PL — CVE-2024-3955: Arbitrary code execution in CraftBeerPi 4
2. NVD — CVE-2024-3955 record
3. PiBrewing/craftbeerpi4 — issue #132
4. PiBrewing/craftbeerpi4 — commit 57572c7, the parameter check
5. CWE-94 — Improper Control of Generation of Code