Skip to content

Fix PatchUtil charAt OOB on truncated new mode / new file mode lines#29624

Closed
moraneus wants to merge 1 commit into
bazelbuild:masterfrom
moraneus:fix-patchutil-truncated-mode-oob
Closed

Fix PatchUtil charAt OOB on truncated new mode / new file mode lines#29624
moraneus wants to merge 1 commit into
bazelbuild:masterfrom
moraneus:fix-patchutil-truncated-mode-oob

Conversation

@moraneus
Copy link
Copy Markdown
Contributor

@moraneus moraneus commented May 22, 2026

Description

PatchUtil.applyInternal calls line.charAt(12) for NEW_MODE lines and line.charAt(17) for NEW_FILE_MODE lines without verifying the line is long enough. A patch file containing a truncated mode line — for example new mode 1 or new file mode 1 — passes the prefix-based line-type classification (because the classifier only checks startsWith("new mode ") / startsWith("new file mode ")) and then crashes in charAt with StringIndexOutOfBoundsException.

That exception is not declared on PatchUtil.apply (which throws IOException and PatchFailedException) and is not caught by the callers (StarlarkRepositoryContext.patch, ModuleFileFunction.applyPatchesToModuleFile). It propagates all the way up through Skyframe's RepositoryFetchFunction as RuntimeException: Unrecoverable error..., hits Bazel's top-level fatal handler, and causes:

FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node
  REPOSITORY_DIRECTORY:@@+crash_ext+crashrepo
Caused by: net.starlark.java.eval.Starlark$UncheckedEvalException:
  StringIndexOutOfBoundsException thrown during Starlark evaluation
Caused by: java.lang.StringIndexOutOfBoundsException:
  Index 12 out of bounds for length 10
    at java.base/java.lang.String.charAt(...)
    at com.google.devtools.build.lib.bazel.repository.decompressor
          .PatchUtil.applyInternal(PatchUtil.java:487)

The Bazel server daemon then exits. Confirmed end-to-end on Bazel 9.1.0 with a 35-byte patch supplied through a Bzlmod module_extension and reproduced on master HEAD.

Reachability

PatchUtil.apply is reached from:

  • http_archive(patches=...) / git_repository(patches=...)
  • single_version_override(patches=...) in MODULE.bazel
  • Bzlmod source.json#patches map (any module in any configured registry, including the Bazel Central Registry)
  • ctx.patch() from any repository_rule / module_extension

All of these accept attacker-influenced patch content. A malicious or compromised module in a registry can ship a 35-byte patch that crashes every Bazel server that resolves it.

Fix

Add a length guard before the charAt(index) so that a truncated mode line produces the declared PatchFailedException instead of an unchecked StringIndexOutOfBoundsException.

case NEW_MODE, NEW_FILE_MODE -> {
    int index = type == LineType.NEW_MODE ? 12 : 17;
    if (line.length() <= index) {
        throw new PatchFailedException(
            "Truncated file mode at line " + (i + 1) + ": " + line);
    }
    char c = line.charAt(index);
    ...
}

Tests added

Two new regression tests in PatchUtilTest covering both index sites:

  • testTruncatedNewFileModeRaisesPatchFailed"new file mode 1" (length 15) at the charAt(17) site.
  • testTruncatedNewModeRaisesPatchFailed"new mode 1" (length 10) at the charAt(12) site.

Both assert the call throws PatchFailedException with "Truncated file mode" in the message.

How to reproduce on master before the fix

Workspace (3 files):

MODULE.bazel:
    module(name = "patchcrash", version = "0.0.0")
    crash = use_extension("//:crash.bzl", "crash_ext")
    use_repo(crash, "crashrepo")

crash.bzl:
    def _impl(ctx):
        ctx.file("BUILD.bazel", "")
        ctx.file("foo.txt", "line1\n")
        ctx.file("crash.patch", "diff --git a/foo b/foo\nnew mode 1\n")
        ctx.patch("crash.patch", strip = 1)
    _crashrule = repository_rule(implementation = _impl)
    def _ext_impl(ctx):
        _crashrule(name = "crashrepo")
    crash_ext = module_extension(implementation = _ext_impl)

BUILD.bazel:
    filegroup(name = "all", srcs = ["@crashrepo//:BUILD.bazel"])

Then bazel build //:all prints the FATAL crash above; the Bazel server PID is gone after the command exits.

With this patch applied the same input fails cleanly with a Truncated file mode at line 2: new mode 1 error and the server stays alive.

Notes

Found via Jazzer fuzzing of PatchUtil.apply.

PatchUtil.applyInternal calls line.charAt(12 or 17) without checking the line length first. A truncated
  "new mode 1" or "new file mode 1" patch line crashes the parser with an uncaught StringIndexOutOfBoundsException that propagates up through Skyframe and terminates the Bazel server daemon. Add a
  length guard so the parser raises the declared PatchFailedException instead.

  Reported via Google OSS-VRP issue 515719440.

Signed-off-by: Moran Omer <moraneus@gmail.com>
@github-actions github-actions Bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels May 22, 2026
@Wyverald
Copy link
Copy Markdown
Member

Thanks for the bug fix.

Per the OSS-VRP team's response, memory-corruption-class findings in OT0 Flagship projects require either a merged upstream patch or an OSS-Fuzz reproduction target before they can be considered for reward; this PR exists to satisfy that requirement.

Just to temper your expectations: this is most definitely not fixing a memory corruption. It's making Bazel return a nicer error message when a patch file is malformed, and that's all it is.

Also, a word of advice: next time you open a PR with the help of an AI agent, don't copy the instructions the agent left for you into the PR description.

@Wyverald Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 22, 2026
@moraneus
Copy link
Copy Markdown
Contributor Author

moraneus commented May 22, 2026 via email

@github-actions github-actions Bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants