From f39f36f2c7f38ecc8c171447de4c65c8cb968640 Mon Sep 17 00:00:00 2001 From: Sergiusz Bazanski Date: Tue, 3 Oct 2017 18:48:53 +0100 Subject: [PATCH 1/2] Fix file missing error message on imports. Trying to convert an import path into a Boost canonical path causes boost to throw an exception if the given file does not exist. Thus, instead of geting to the 'File not found' error, we instead got into the cath-all handler for 'Unknown exception in read callback'. This change rearranges the file checks to happen before we create a canonical Boost path. It also drive-by removes the unnecessary 'else' block, as the body of the if is a guard-like return block. --- solc/CommandLineInterface.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 8f81e7994..93203de60 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -700,7 +700,13 @@ bool CommandLineInterface::processInput() try { auto path = boost::filesystem::path(_path); + if (!boost::filesystem::exists(path)) + return ReadCallback::Result{false, "File not found."}; + auto canonicalPath = boost::filesystem::canonical(path); + if (!boost::filesystem::is_regular_file(canonicalPath)) + return ReadCallback::Result{false, "Not a valid file."}; + bool isAllowed = false; for (auto const& allowedDir: m_allowedDirectories) { @@ -716,16 +722,10 @@ bool CommandLineInterface::processInput() } if (!isAllowed) return ReadCallback::Result{false, "File outside of allowed directories."}; - else if (!boost::filesystem::exists(path)) - return ReadCallback::Result{false, "File not found."}; - else if (!boost::filesystem::is_regular_file(canonicalPath)) - return ReadCallback::Result{false, "Not a valid file."}; - else - { - auto contents = dev::readFileAsString(canonicalPath.string()); - m_sourceCodes[path.string()] = contents; - return ReadCallback::Result{true, contents}; - } + + auto contents = dev::readFileAsString(canonicalPath.string()); + m_sourceCodes[path.string()] = contents; + return ReadCallback::Result{true, contents}; } catch (Exception const& _exception) { From c15cb6cc7ac68e539dd3969e614be52e9a943ec7 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 5 Apr 2018 14:25:14 +0200 Subject: [PATCH 2/2] Prevent information about file existence outside the allowed paths to leak by mimicing boost::filesystem::weakly_canonical. --- Changelog.md | 1 + libdevcore/CommonIO.cpp | 20 ++++++++++++++++++++ libdevcore/CommonIO.h | 5 +++++ solc/CommandLineInterface.cpp | 14 +++++++------- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/Changelog.md b/Changelog.md index d6860bdf9..c2ef7210a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -20,6 +20,7 @@ Bugfixes: * Code Generator: Bugfix in modifier lookup in libraries. * Code Generator: Implement packed encoding of external function types. * Code Generator: Treat empty base constructor argument list as not provided. + * Commandline interface: Fix error messages for imported files that do not exist. * Commandline interface: Support ``--evm-version constantinople`` properly. * DocString Parser: Fix error message for empty descriptions. * Standard JSON: Support ``constantinople`` as ``evmVersion`` properly. diff --git a/libdevcore/CommonIO.cpp b/libdevcore/CommonIO.cpp index 6526baf97..0063a8d46 100644 --- a/libdevcore/CommonIO.cpp +++ b/libdevcore/CommonIO.cpp @@ -167,3 +167,23 @@ int dev::readStandardInputChar() DisableConsoleBuffering disableConsoleBuffering; return cin.get(); } + +boost::filesystem::path dev::weaklyCanonicalFilesystemPath(boost::filesystem::path const &_path) +{ + if (boost::filesystem::exists(_path)) + return boost::filesystem::canonical(_path); + else + { + boost::filesystem::path head(_path); + boost::filesystem::path tail; + for (auto it = --_path.end(); !head.empty(); --it) + { + if (boost::filesystem::exists(head)) + break; + tail = (*it) / tail; + head.remove_filename(); + } + head = boost::filesystem::canonical(head); + return head / tail; + } +} diff --git a/libdevcore/CommonIO.h b/libdevcore/CommonIO.h index 3ecdb4c38..9ba68e745 100644 --- a/libdevcore/CommonIO.h +++ b/libdevcore/CommonIO.h @@ -25,6 +25,7 @@ #include #include +#include #include "Common.h" namespace dev @@ -57,4 +58,8 @@ std::string toString(_T const& _t) return o.str(); } +/// Partial implementation of boost::filesystem::weakly_canonical (available in boost>=1.60). +/// Should be replaced by the boost implementation as soon as support for boost<1.60 can be dropped. +boost::filesystem::path weaklyCanonicalFilesystemPath(boost::filesystem::path const &_path); + } diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 93203de60..4da394b2f 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -700,13 +700,7 @@ bool CommandLineInterface::processInput() try { auto path = boost::filesystem::path(_path); - if (!boost::filesystem::exists(path)) - return ReadCallback::Result{false, "File not found."}; - - auto canonicalPath = boost::filesystem::canonical(path); - if (!boost::filesystem::is_regular_file(canonicalPath)) - return ReadCallback::Result{false, "Not a valid file."}; - + auto canonicalPath = weaklyCanonicalFilesystemPath(path); bool isAllowed = false; for (auto const& allowedDir: m_allowedDirectories) { @@ -723,6 +717,12 @@ bool CommandLineInterface::processInput() if (!isAllowed) return ReadCallback::Result{false, "File outside of allowed directories."}; + if (!boost::filesystem::exists(canonicalPath)) + return ReadCallback::Result{false, "File not found."}; + + if (!boost::filesystem::is_regular_file(canonicalPath)) + return ReadCallback::Result{false, "Not a valid file."}; + auto contents = dev::readFileAsString(canonicalPath.string()); m_sourceCodes[path.string()] = contents; return ReadCallback::Result{true, contents};