`base64_decode`: integer underflow on invalid padding leads to out-of-bounds read
- Dominant language
- C
- Stars
- 4.1k
- Forks
- 444
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 43
Description
A few weeks ago I did
1. In https://claude.ai/ web interface for Opus 4.8 (high) type
```
I have written many git commits to the crun project.
Could you take a look at the crun source code (https://github.com/containers/crun)
and check if you can find bugs or suggest improvements?
```
Claude Opus 4.8 wrote a lot of text.
2. Answer some follow-up questions. Essentially I just asked Claude to continue looking in the source code files it suggested. There were also some other small bugs that Claude found.
3. Type
```
draft a public GitHub issue
```
click me to show issue Claude drafted
# `base64_decode`: integer underflow on invalid padding leads to out-of-bounds read
## Summary
`base64_decode()` in `src/libcrun/utils.c` does not validate the number of `=`
padding characters in a 4-character group, and it never resets its `pad`
counter between groups. An input group containing three or four `=` characters
(e.g. `"===="`) makes `pad` exceed 3, so the length update
```c
(*nbytes) += 3 - pad;
```
underflows the **unsigned** `*nbytes` to a value near `SIZE_MAX`. The single
caller, `libcrun_copy_seccomp()`, then passes that length to `safe_write()`,
producing a large out-of-bounds read from a small heap buffer (crash /
`SIGSEGV`).
> **Note on scope:** this is *not* being reported as a security vulnerability.
> The only trigger is the `run.oci.seccomp_bpf_data` annotation, which comes
> from `config.json`. Per crun's `SECURITY.md`, `config.json` (including
> annotations) is trusted input, and an attacker who controls it can already
> disable seccomp outright — so this grants no additional capability. It is
> filed here as an ordinary robustness/correctness bug: malformed input should
> fail cleanly rather than underflow a length, and it currently trips crun's
> own AddressSanitizer CI.
## Root cause
In `base64_decode()`:
* `pad` is declared once and incremented for every `=` seen, but is never reset
between 4-character groups.
* There is no check that `pad <= 2` (the maximum valid padding for a base64
group), so `3 - pad` can be negative and wraps the unsigned `*nbytes`.
## Reproduction
`libcrun_copy_seccomp()` (`src/libcrun/seccomp.c`) does:
```c
in_size = strlen (b64_bpf);
bpf_data = xmalloc (in_size + 1);
consumed = base64_decode (b64_bpf, in_size, bpf_data, in_size, &size);
if (consumed != (int) in_size)
return crun_make_error (err, 0, "invalid seccomp BPF data");
ret = safe_write (gen_ctx->fd, "seccomp fd", bpf_data, size, err);
```
With `b64_bpf = "===="`:
* the single group has `pad == 4`, so `size` (`*nbytes`) becomes
`18446744073709551615` (`SIZE_MAX`);
* `consumed == 4 == in_size`, so the `consumed != in_size` guard **passes**;
* `safe_write(..., bpf_data, SIZE_MAX)` reads far past the 5-byte `bpf_data`
buffer.
Reached from container config:
`find_annotation(container, "run.oci.seccomp_bpf_data")`
→ `setup_seccomp()` → `seccomp_generation()` → `libcrun_copy_seccomp()`
(`src/libcrun/container.c`).
Minimal standalone reproducer against the current decoder:
```c
char out[8];
size_t n = 0;
int consumed = base64_decode ("====", 4, out, 4, &n);
/* consumed == 4, n == (size_t) -1 -> caller then over-reads `out` */
```
Compiling this under `-fsanitize=address` reports a heap-buffer-overflow at the
subsequent read of `n` bytes.
## Suggested fix
Reject groups with more than two padding characters (invalid base64 anyway) and
reset `pad` for each group, so malformed input fails cleanly via the existing
`consumed != in_size` check instead of underflowing the length:
```c
if (i == 4)
{
if (UNLIKELY (pad > 2))
return -1;
*optr++ = (data[0] << 2) | ((data[1] & 0x30) >> 4);
*optr++ = ((data[1] & 0xf) << 4) | ((data[2] & 0x3c) >> 2);
*optr++ = ((data[2] & 0x3) << 6) | data[3];
(*nbytes) += 3 - pad;
}
...
i = 0;
pad = 0;
```
With this change, valid base64 continues to decode unchanged (verified against
the RFC 4648 vectors), `"===="` and similar are rejected, and 3M randomized /
malformed inputs run clean under ASan/UBSan with no underflow and no
out-of-bounds access.
I have a patch ready for this plus a unit test for `base64_decode` (valid
vectors and the `"===="` regression) in `tests/tests_libcrun_utils.c`; happy to
open a PR.
## Environment
* Component: `src/libcrun/utils.c` (`base64_decode`), triggered via
`src/libcrun/seccomp.c` (`libcrun_copy_seccomp`)
* Affects: current `main`
Contributor guide
Research direction
Start with base64_decode in src/libcrun/utils.c and trace its caller libcrun_copy_seccomp in src/libcrun/seccomp.c. Run the existing tests and inspect tests/tests_libcrun_utils.c, using the provided malformed-padding reproducer and ASan/UBSan checks. Done means invalid padding is rejected without length underflow or an out-of-bounds read, while valid RFC 4648 vectors still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100