Skip to content
This repository has been archived by the owner on Jun 19, 2023. It is now read-only.

Fix ipfs/go-ipfs#4224 #25

Closed
wants to merge 1 commit into from
Closed

Conversation

jbarthelmes
Copy link

@jbarthelmes jbarthelmes commented Feb 12, 2020

Lift (arbitrary?) restriction on file location.

Original author: @whyrusleeping

@jbarthelmes
Copy link
Author

jbarthelmes commented Feb 12, 2020

ipfs/kubo#4224 (comment) @Stebalien

The only remaining attack vector is confirmation: verifying that some file exists at some path. Unfortunately, that's easily exploitable with guess and check: guess the first byte, guess the second byte, etc.

I don't understand this yet.

@Stebalien
Copy link
Member

If an attacker could access the go-ipfs API, it could use the filestore to read an arbitrary file from the disk.

@Stebalien Stebalien closed this Feb 12, 2020
@jbarthelmes
Copy link
Author

jbarthelmes commented Feb 12, 2020

@Stebalien

If an attacker could access the go-ipfs API, it could use the filestore to read an arbitrary file from the disk.

I still don't see the problem that the deleted 3 lines of code solve.

access the go-ipfs API

Which API do you mean? I'm thinking of these scenarios:

  1. Local privilege escalation: attacker gains same read access as ipfs daemon user. Solution: restrict that user's rights. Access to $HOME is already enforced by the OS.
  2. Remote attacker uses HTTP API to read from file system. Solution: Fix your network security.

Either way, restricting access to $HOME by default does not protect sensitive files like .ssh/id_* or .ipfs/config.

@Stebalien
Copy link
Member

Let's discuss this on the main issue. The question isn't how to lift this limitation, it's should we lift it and when.

@@ -281,9 +281,6 @@ func (f *FileManager) putTo(b *posinfo.FilestoreNode, to putter) error {
if !f.AllowFiles {
return ErrFilestoreNotEnabled
}
if !filepath.HasPrefix(b.PosInfo.FullPath, f.root) { //nolint:staticcheck
return fmt.Errorf("cannot add filestore references outside ipfs root (%s)", f.root)
}

p, err := filepath.Rel(f.root, b.PosInfo.FullPath)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be changed too, so that dobj.FilePath saves an absolute path

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants