Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--allow-paths does not resolve relative paths correctly #4623

Closed
lorenzb opened this issue Jul 30, 2018 · 4 comments · Fixed by #11688
Closed

--allow-paths does not resolve relative paths correctly #4623

lorenzb opened this issue Jul 30, 2018 · 4 comments · Fixed by #11688
Assignees
Labels
bug 🐛 should compile without error Error is reported even though it shouldn't. Source is fine.

Comments

@lorenzb
Copy link

lorenzb commented Jul 30, 2018

I ran into some ergonomics issues related to --allow-paths and symlinks.

Problem description

Say I have two contracts Foo (in foo.sol) and Bar (in bar/bar.sol) and assume that foo.sol imports bar/bar.sol.

This obviously works:

$ tree -l
.
├── bar
│   └── bar.sol
└── foo.sol

$ solc foo.sol --metadata
[...]

If we move bar.sol behind a symlink, everything still works:

$ tree -l
.
├── bar -> bar2
│   └── bar.sol
├── bar2
│   └── bar.sol
└── foo.sol

$ solc foo.sol --metadata
[...]

... or not, depending on where the symlink points to:

$ tree -l
.
├── bar -> ../bar2/
│   └── bar.sol
└── foo.sol

$ solc foo.sol --metadata
foo.sol:1:1: Error: Source "bar/bar.sol" not found: File outside of allowed directories.

Weird, maybe I just need to add bar as an allowed directory?

$ solc foo.sol --metadata --allow-paths bar
foo.sol:1:1: Error: Source "bar/bar.sol" not found: File outside of allowed directories.

Nope, not working. Maybe I should try resolving the symlink?

$ solc foo.sol --metadata --allow-paths ../bar2
foo.sol:1:1: Error: Source "bar/bar.sol" not found: File outside of allowed directories.

Hmm, that doesn't work. (See also this issue.)
But this does:

$ solc foo.sol --metadata --allow-paths /absolute/path/to/bar2
[...]

Note that solc generates identical metadata in all three cases that compile:

======= bar/bar.sol:Bar =======
Metadata: 
{"compiler":{"version":"0.4.24+commit.e67f0147"},"language":"Solidity","output":{"abi":[],"devdoc":{"methods":{}},"userdoc":{"methods":{}}},"settings":{"compilationTarget":{"bar/bar.sol":"Bar"},"evmVersion":"byzantium","libraries":{},"optimizer":{"enabled":false,"runs":200},"remappings":[]},"sources":{"bar/bar.sol":{"keccak256":"0x126b05c69a314dd4dc83ce9e9ffcb9832a1cb228f8d594b35ee9845d6cfa9a44","urls":["bzzr://09984546db62920903b6d6f760674a79af34d16306b6a963e19ad4ae35151d75"]}},"version":1}

======= foo.sol:Foo =======
Metadata: 
{"compiler":{"version":"0.4.24+commit.e67f0147"},"language":"Solidity","output":{"abi":[],"devdoc":{"methods":{}},"userdoc":{"methods":{}}},"settings":{"compilationTarget":{"foo.sol":"Foo"},"evmVersion":"byzantium","libraries":{},"optimizer":{"enabled":false,"runs":200},"remappings":[]},"sources":{"bar/bar.sol":{"keccak256":"0x126b05c69a314dd4dc83ce9e9ffcb9832a1cb228f8d594b35ee9845d6cfa9a44","urls":["bzzr://09984546db62920903b6d6f760674a79af34d16306b6a963e19ad4ae35151d75"]},"foo.sol":{"keccak256":"0x32ef0f192269bf14bcb7466f6f069813a9dc371eb9dca5d87426e8554bed0433","urls":["bzzr://ae450631ec9fe187bdfcbf2f01266637fddbe2b2ade5060ea50810e47a7f6703"]}},"version":1}

Suggestions

Can we improve this situation to make it more developer friendly?

Here are a few suggestions for resolving this issue:

  1. Follow symlinks (from what I gather, symlinks are currently not supported for security reasons. I must confess that I don't understand those reasons. Either you can trust your FS or you can't.)
  2. Allow --allow-paths to:
    2.1 resolve symlinks
    2.2 resolve relative paths
  3. If (1) and (2) are too controversial, at least:
    3.1 document the behavior
    3.2 detect that compilation failed due to a symlink/relative path, warn the user, and point them to the docs
@relyt29
Copy link

relyt29 commented Jul 30, 2018

  1. Add a "force-follow-symlinks-unsafe-who-needs-security-anyways" option

(Edit: note I don't think this option is the best idea, just wanted to make it known that it is also an approach)

@lorenzb lorenzb changed the title Ergonomics of --allow-paths and symlink handling Ergonomics of --allow-paths and symlink/relative path handling Jul 30, 2018
@chriseth
Copy link
Contributor

chriseth commented Aug 3, 2018

Note that the metadata is identical because the physical paths on the machine you compile from are irrelevant for the metadata, only the virtual directory structure (with foo.sol at the root and bar/bar.sol inside that) is visible.

As far as I understand the problem, this just looks like relative paths are not properly taken into account for --allowed-paths, so basically my proposal would be to fix it according to (2).

@chriseth chriseth changed the title Ergonomics of --allow-paths and symlink/relative path handling [CLI] Ergonomics of --allow-paths and symlink/relative path handling Aug 31, 2020
@cameel
Copy link
Member

cameel commented Mar 3, 2021

The reason symlinks are not followed is simply that it would make the whole feature pointless. You use it to whitelist specific directories and be sure that nothing outside them cannot be included in the contract. If the compiler followed symlinks, this could be trivially by passed by placing a symlink to any file on your filesystem inside one of the whitelisted directories.

@cameel cameel self-assigned this Mar 3, 2021
nkuba added a commit to keep-network/keep-ecdsa that referenced this issue May 12, 2021
solc doesn't support symbolic links that are made in `node_modules` by `npm link`
command. We need to update the `--allow-paths` value to be the parent directory
that is assumed to contain both current project and dependent project.
Ref: ethereum/solidity#4623
nkuba added a commit to keep-network/keep-ecdsa that referenced this issue May 20, 2021
solc doesn't support symbolic links that are made in `node_modules` by `npm link`
command. We need to update the `--allow-paths` value to be the parent directory
that is assumed to contain both current project and dependent project.
Ref: ethereum/solidity#4623
@cameel cameel changed the title [CLI] Ergonomics of --allow-paths and symlink/relative path handling --allow-paths does not resolve relative paths correctly Jun 2, 2021
@cameel
Copy link
Member

cameel commented Jun 2, 2021

Just to clarify this issue: it's not really a problem with symlinks, relative paths without symlinks do not work properly either:

$ tree -l
.
├── bar2
│   └── bar.sol
└── foo.sol

$ solc foo.sol --metadata --allow-paths ../bar2
[...]

@cameel cameel added the should compile without error Error is reported even though it shouldn't. Source is fine. label Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 should compile without error Error is reported even though it shouldn't. Source is fine.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants