镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Extract map archives when the base path is relative - #148

Open
FrogAi wants to merge 1 commit into
pfeiferj:mainfrom
FrogAi:fix-offdevice-extract
Open

FrogAi wants to merge 1 commit into
pfeiferj:mainfrom
FrogAi:fix-offdevice-extract

Conversation

@FrogAi

@FrogAi FrogAi commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Map downloads extract nothing on any machine without "/data/media/0" since aa04ba4. "GetBaseOpPath()" returns "." there, and "safeJoin" then rejects every archive entry as escaping the destination, so a download finishes with 0 of N files and no error shown. On a comma device the base path is "/data/media/0/osm" and nothing changes.

This keeps the tar-slip check but does it with "filepath.Rel" and "filepath.IsLocal" instead of a string prefix, which works for relative and absolute bases.

Base path Entry Before After
"." "offline/46/-124/data" Rejected Allowed
"/data/media/0/osm" "offline/46/-124/data" Allowed Allowed
Both "../../../etc/passwd", "offline/../../../../tmp/evil", "a/../../b" Rejected Rejected
Both "/etc/shadow" Allowed under the base Allowed under the base (unchanged)
Audit report

Cause

"safeJoin" builds "filepath.Clean(base) + "/"" and checks that the cleaned target starts with it. "filepath.Clean(".")" is ".", so the prefix becomes "./", but "filepath.Join(".", "offline/46/-124/")" is "offline/46/-124" with no "./" in front. Every real entry fails the prefix check. The existing "safeJoin" tests all use "/data/media/0/osm" as the base, so they couldn't catch it.

Fix

"filepath.Rel(base, target)" gives the target's path relative to the base, and "filepath.IsLocal" rejects anything that leaves it ("..", "../x" and so on). An absolute entry name like "/etc/shadow" is still joined under the base, so it stays allowed the way "TestSafeJoinAllowsNormalEntries" expects. "IsLocal" has been in Go since 1.20 and go.mod is 1.25.1.

Testing

  • "go vet ./..." and "go test ./..." pass with "settings/download_validate_test.go" unchanged, in "golang:1.26.2-bookworm".
  • A probe (not committed) ran 14 entry names against three bases (".", "/data/media/0/osm" and "rel/base"). Every normal entry is allowed under all three, and every escaping entry is rejected under all three. With the device base, every result matches main.
  • An end-to-end probe in a container without "/data/media/0" extracted a real archive built the same way "compress_offline.sh" builds them. main returned "archive entry escapes destination: offline/46/-124/" and extracted nothing. With this change the tile was written.

Not covered

I didn't add a regression test with base "." to the existing table, to keep the change to the fix itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant