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

Flaws in Cast Downloading Contract Source Code #4896

Closed
1 of 2 tasks
Hellobloc opened this issue May 8, 2023 · 3 comments · Fixed by foundry-rs/block-explorers#32
Closed
1 of 2 tasks

Flaws in Cast Downloading Contract Source Code #4896

Hellobloc opened this issue May 8, 2023 · 3 comments · Fixed by foundry-rs/block-explorers#32
Labels
T-bug Type: bug

Comments

@Hellobloc
Copy link

Component

Cast

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

No response

What command(s) is the bug in?

cast etherscan

Operating System

Windows

Describe the bug

Introduction

cast supports downloading contract source code locally via etherscan. cast uses the rs library of ethers-etherscan to handle part of the download logic. The path traversal vulnerability is well guarded in the ethers-etherscan library, but the operation of automatically adding the .sol suffix causes the tool to have a possible new risk.
https://github.com/foundry-rs/foundry/blob/master/cast/src/lib.rs#L19
https://sourcegraph.com/crates/ethers-etherscan@9e675141953f88a190ac883ad02841370836d35c/-/blob/src/source_tree.rs?L18-36

impl SourceTree {
    /// Expand the source tree into the provided directory.  This method sanitizes paths to ensure
    /// that no directory traversal happens.
    pub fn write_to(&self, dir: &Path) -> Result<()> {
        create_dir_all(dir)?;
        for entry in &self.entries {
            let mut sanitized_path = sanitize_path(&entry.path);
            if sanitized_path.extension().is_none() {
                sanitized_path.set_extension("sol");
            }
            let joined = dir.join(sanitized_path);
            if let Some(parent) = joined.parent() {
                create_dir_all(parent)?;
                std::fs::write(joined, &entry.contents)?;
            }
        }
        Ok(())
    }
}

Risk

Specifically, we can build two contract files, Attack and Attack.sol, in Etherscan, which makes it possible for the two files to have the same filename and be overwritten when the source code of etherscan is downloaded locally by cast. This feature can be used to implement some honeypot contracts, where we can make some backdoor source code content overwritten and not visible through the source code.

Attack Case

Here we can try to allow the following command to verify the problem

cast etherscan-source 0x277A372cD28bA6B62DCB8C0D9491d6BE26a0D216 -c goerli --etherscan-api-key AA -d test_file

image

Here we can easily find that our file directory only has an almost empty Token.sol file, but the actual Token.sol is overwritten. In a real scenario, we can build some honeypot contracts to defraud users by designing the Token.sol contract to be more complex making it difficult to detect the overwriting happening.

Recommendation

We recommend removing the appending of filenames (.sol) whenever possible, or giving some warning notes and trying to detect such issues and warn about them.

@Hellobloc Hellobloc added the T-bug Type: bug label May 8, 2023
@gakonst gakonst added this to Foundry May 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry May 8, 2023
@mattsse
Copy link
Member

mattsse commented May 8, 2023

tysm for this.

We recommend removing the appending of filenames (.sol) whenever possible,

This seems like the right fix, so we should remove the set_extension?

@slendermaan
Copy link

also could just add a check!if there are some file don't have '.sol' in the end of name!
we can just alert it or ban this fetch!

@slendermaan
Copy link

i'm @Hellobloc , this is another account! sorry for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants