Skip to content

Attachment upload reads outside the os.Root that checked it #186

Description

@willkg

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.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions