Attachment upload reads outside the os.Root that checked it
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 55/100
Research direction
Trace the image path through internal/convert/images.go and internal/project/project.go, then inspect LocalAttachment, fileChecksum, and upload handling in internal/client/client.go. Also check how attachment-upload constructs LocalAttachment values. Done means checksum and upload read through the same os.Root that performed the check, with the no-read-outside-root scenario covered by tests.
Written by the indexing model from the issue text.
Description
markfluence checks each image through an os.Root but uploads it through an ordinary path, so the check and the read are two different lookups.
The converter checks an image with root.FS.Lstat(rootRel) (internal/convert/images.go). root.FS is an os.Root bounded to the documentation root (internal/project/project.go), so it refuses a symlinked leaf and an escape through a symlinked directory. But the LocalAttachment the converter hands on carries Path: filepath.Join(r.root.Dir, rootRel). fileChecksum and the upload then call os.Open on that path (internal/client/client.go), outside the os.Root.
So if a directory on that path is replaced with a symlink between the check and the upload, the uploaded bytes can come from outside the root. S2 (no-read-outside-root) is meant to rule that out. This is a race, not a static hole: no layout of files on disk triggers it by itself.
A likely fix is to open the file through the same os.Root that checked it, instead of by absolute path. For example, carry the root-relative path and the *os.Root (or an opener) on LocalAttachment. attachment-upload builds LocalAttachment values too, so it would need the same treatment.
- Dominant language
- Go
- Stars
- 2
- Forks
- 0
- Avg merge
- 2h 50m
- Merged PRs (30d)
- 52
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from mozilla/markfluence
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
mozilla/markfluence#185 ·
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
mozilla/markfluence#184 ·
-
bug
Difficulty 3/5 1-2 days Newbie friendliness 55/100
mozilla/markfluence#181 · 1 comment ·
-
bug
Difficulty 5/5 Over a week Newbie friendliness 48/100
mozilla/markfluence#163 ·
-
enhancement
Difficulty 5/5 Over a week Newbie friendliness 25/100
mozilla/markfluence#162 ·
All issues in mozilla/markfluence
Similar issues
-
bug github_actions
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
registrystack/registry-stack#1393 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
JakeChampion/lang#10213 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
oasisprotocol/oasis-sdk#2523 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100