fix(contract-verifier): allow relative imports (#4761)

Author: coffeexcoin

core/lib/contract_verifier/src/compilers/mod.rs

@@ -72,15 +72,18 @@ pub(crate) fn validate_source_paths(
 }
 
 /// Returns `true` if `source` contains an `import` directive whose path is absolute (`/…`)
-/// or starts with a parent-directory traversal (`../…`).  Both forms let the compiler
-/// resolve imports against the host filesystem and leak file contents in error messages.
+/// or uses a `file://` URL. These forms let the compiler resolve imports against the host
+/// filesystem and potentially leak file contents in error messages.
+///
+/// Relative imports containing `../` are allowed: they are standard Solidity practice and
+/// remain sandboxed by `validate_source_paths()` plus the empty compiler search directory.
 pub(crate) fn has_dangerous_imports(source: &str) -> bool {
     // Covers all Solidity import forms:
     //   import "/path";
-    //   import "../path";
     //   import {X} from "/path";
     //   import * as X from "/path";
-    let re = Regex::new(r#"\bimport\b[^;]*?["'](/|\.\.)"#).unwrap();
+    //   import "file:///path";
+    let re = Regex::new(r#"\bimport\b[^;]*?["'](?:/|file://)"#).unwrap();
     re.is_match(source)
 }
 
@@ -114,6 +117,30 @@ pub(crate) fn sanitize_compiler_stderr(stderr: &str) -> String {
         .join("\n")
 }
 
+#[cfg(test)]
+mod tests {
+    use super::has_dangerous_imports;
+
+    #[test]
+    fn allows_relative_parent_imports() {
+        let source = r#"
+            import {ContextUpgradeable} from "../utils/ContextUpgradeable.sol";
+            import {Hashes} from "./Hashes.sol";
+        "#;
+
+        assert!(
+            !has_dangerous_imports(source),
+            "relative imports within the submitted source tree must be allowed"
+        );
+    }
+
+    #[test]
+    fn rejects_absolute_imports() {
+        assert!(has_dangerous_imports(r#"import "/etc/shadow";"#));
+        assert!(has_dangerous_imports(r#"import "file:///etc/shadow";"#));
+    }
+}
+
 /// Users may provide either just contract name or source file name and contract name joined with ":".
 fn process_contract_name(original_name: &str, extension: &str) -> (String, String) {
     if let Some((file_name, contract_name)) = original_name.rsplit_once(':') {

core/lib/contract_verifier/src/compilers/solc.rs

@@ -49,7 +49,7 @@ impl Solc {
             SourceCodeData::SolSingleFile(source_code) => {
                 if has_dangerous_imports(&source_code) {
                     return Err(ContractVerifierError::InvalidSourcePath(
-                        "import with absolute or traversal path".to_owned(),
+                        "import with absolute path".to_owned(),
                     ));
                 }
                 let source = Source {
@@ -85,7 +85,7 @@ impl Solc {
                 for source in compiler_input.sources.values() {
                     if has_dangerous_imports(&source.content) {
                         return Err(ContractVerifierError::InvalidSourcePath(
-                            "import with absolute or traversal path".to_owned(),
+                            "import with absolute path".to_owned(),
                         ));
                     }
                 }
@@ -123,6 +123,105 @@ impl Solc {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use zksync_types::contract_verification::api::CompilerVersions;
+
+    use super::*;
+
+    #[test]
+    fn build_input_allows_relative_parent_imports_in_standard_json() {
+        let input = serde_json::json!({
+            "language": "Solidity",
+            "sources": {
+                "src/Counter.sol": {
+                    "content": r#"
+                        pragma solidity ^0.8.20;
+                        import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+
+                        contract Counter is OwnableUpgradeable {
+                            function initialize(address owner) external initializer {
+                                __Ownable_init(owner);
+                            }
+                        }
+                    "#,
+                },
+                "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol": {
+                    "content": r#"
+                        pragma solidity ^0.8.20;
+                        import "../utils/ContextUpgradeable.sol";
+                        import "@openzeppelin/contracts/proxy/utils/Initializable.sol";
+
+                        abstract contract OwnableUpgradeable is Initializable, ContextUpgradeable {
+                            address private _owner;
+
+                            function __Ownable_init(address initialOwner) internal onlyInitializing {
+                                _owner = initialOwner;
+                            }
+                        }
+                    "#,
+                },
+                "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol": {
+                    "content": r#"
+                        pragma solidity ^0.8.20;
+                        import "@openzeppelin/contracts/proxy/utils/Initializable.sol";
+
+                        abstract contract ContextUpgradeable is Initializable {
+                            function _msgSender() internal view virtual returns (address) {
+                                return msg.sender;
+                            }
+                        }
+                    "#,
+                },
+                "@openzeppelin/contracts/proxy/utils/Initializable.sol": {
+                    "content": r#"
+                        pragma solidity ^0.8.20;
+
+                        abstract contract Initializable {
+                            modifier initializer() {
+                                _;
+                            }
+
+                            modifier onlyInitializing() {
+                                _;
+                            }
+                        }
+                    "#,
+                },
+            },
+            "settings": {
+                "optimizer": {
+                    "enabled": true,
+                },
+            },
+        });
+        let req = VerificationIncomingRequest {
+            contract_address: Default::default(),
+            source_code_data: SourceCodeData::StandardJsonInput(input.as_object().unwrap().clone()),
+            contract_name: "src/Counter.sol:Counter".to_owned(),
+            compiler_versions: CompilerVersions::Solc {
+                compiler_solc_version: "0.8.26".to_owned(),
+                compiler_zksolc_version: None,
+            },
+            optimization_used: true,
+            optimizer_mode: None,
+            constructor_arguments: Default::default(),
+            is_system: false,
+            force_evmla: false,
+            evm_specific: Default::default(),
+        };
+
+        let built = Solc::build_input(req).expect("relative parent imports should be allowed");
+
+        assert_eq!(built.file_name, "src/Counter.sol");
+        assert_eq!(built.contract_name, "Counter");
+        assert!(built
+            .standard_json
+            .sources
+            .contains_key("@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"));
+    }
+}
+
 #[async_trait]
 impl Compiler<SolcInput> for Solc {
     async fn compile(
@@ -141,6 +240,7 @@ impl Compiler<SolcInput> for Solc {
             .arg("--standard-json")
             .arg("--allow-paths")
             .arg(compile_dir.path())
+            .current_dir(compile_dir.path())
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
             .stderr(Stdio::piped())

core/lib/contract_verifier/src/compilers/zksolc.rs

@@ -101,7 +101,7 @@ impl ZkSolc {
             SourceCodeData::SolSingleFile(source_code) => {
                 if has_dangerous_imports(&source_code) {
                     return Err(ContractVerifierError::InvalidSourcePath(
-                        "import with absolute or traversal path".to_owned(),
+                        "import with absolute path".to_owned(),
                     ));
                 }
                 let source = Source {
@@ -138,7 +138,7 @@ impl ZkSolc {
                 for source in compiler_input.sources.values() {
                     if has_dangerous_imports(&source.content) {
                         return Err(ContractVerifierError::InvalidSourcePath(
-                            "import with absolute or traversal path".to_owned(),
+                            "import with absolute path".to_owned(),
                         ));
                     }
                 }