From e9365e93fcef5298ab960aac9d71d2fa0dd64303 Mon Sep 17 00:00:00 2001 From: ssi91 Date: Fri, 25 Aug 2023 21:41:24 -0400 Subject: [PATCH 1/3] add some test-cases for unverified library addresses --- .../input.json | 33 +++++++++ .../output.json | 12 ++++ .../input.json | 33 +++++++++ .../output.json | 12 ++++ .../input.json | 33 +++++++++ .../output.json | 12 ++++ test/solc/CommandLineParser.cpp | 70 +++++++++++++++++++ 7 files changed, 205 insertions(+) create mode 100644 test/cmdlineTests/linking_standard_solidity_empty_address/input.json create mode 100644 test/cmdlineTests/linking_standard_solidity_empty_address/output.json create mode 100644 test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/input.json create mode 100644 test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/output.json create mode 100644 test/cmdlineTests/linking_standard_solidity_invalid_length_address/input.json create mode 100644 test/cmdlineTests/linking_standard_solidity_invalid_length_address/output.json diff --git a/test/cmdlineTests/linking_standard_solidity_empty_address/input.json b/test/cmdlineTests/linking_standard_solidity_empty_address/input.json new file mode 100644 index 000000000..4afdf77e5 --- /dev/null +++ b/test/cmdlineTests/linking_standard_solidity_empty_address/input.json @@ -0,0 +1,33 @@ +{ + "language": "Solidity", + "sources": { + "A": { + "content": " + // SPDX-License-Identifier: GPL-3.0 + pragma solidity >=0.0; + + library L { + function f() external {} + } + + contract C { + function foo() public { + L.f(); + } + } + " + } + }, + "settings": { + "libraries": { + "A": { + "L": "" + } + }, + "outputSelection": { + "*": { + "C": ["evm.bytecode.object", "evm.bytecode.linkReferences"] + } + } + } +} diff --git a/test/cmdlineTests/linking_standard_solidity_empty_address/output.json b/test/cmdlineTests/linking_standard_solidity_empty_address/output.json new file mode 100644 index 000000000..f8cc5ed1f --- /dev/null +++ b/test/cmdlineTests/linking_standard_solidity_empty_address/output.json @@ -0,0 +1,12 @@ +{ + "errors": + [ + { + "component": "general", + "formattedMessage": "Library address is not prefixed with \"0x\".", + "message": "Library address is not prefixed with \"0x\".", + "severity": "error", + "type": "JSONError" + } + ] +} diff --git a/test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/input.json b/test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/input.json new file mode 100644 index 000000000..88447c993 --- /dev/null +++ b/test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/input.json @@ -0,0 +1,33 @@ +{ + "language": "Solidity", + "sources": { + "A": { + "content": " + // SPDX-License-Identifier: GPL-3.0 + pragma solidity >=0.0; + + library L { + function f() external {} + } + + contract C { + function foo() public { + L.f(); + } + } + " + } + }, + "settings": { + "libraries": { + "A": { + "L": "0x123456789012345678901234567890123456789T" + } + }, + "outputSelection": { + "*": { + "C": ["evm.bytecode.object", "evm.bytecode.linkReferences"] + } + } + } +} diff --git a/test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/output.json b/test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/output.json new file mode 100644 index 000000000..da02e58c7 --- /dev/null +++ b/test/cmdlineTests/linking_standard_solidity_invalid_address_supplied/output.json @@ -0,0 +1,12 @@ +{ + "errors": + [ + { + "component": "general", + "formattedMessage": "Invalid library address (\"0x123456789012345678901234567890123456789T\") supplied.", + "message": "Invalid library address (\"0x123456789012345678901234567890123456789T\") supplied.", + "severity": "error", + "type": "JSONError" + } + ] +} diff --git a/test/cmdlineTests/linking_standard_solidity_invalid_length_address/input.json b/test/cmdlineTests/linking_standard_solidity_invalid_length_address/input.json new file mode 100644 index 000000000..9aa91a07c --- /dev/null +++ b/test/cmdlineTests/linking_standard_solidity_invalid_length_address/input.json @@ -0,0 +1,33 @@ +{ + "language": "Solidity", + "sources": { + "A": { + "content": " + // SPDX-License-Identifier: GPL-3.0 + pragma solidity >=0.0; + + library L { + function f() external {} + } + + contract C { + function foo() public { + L.f(); + } + } + " + } + }, + "settings": { + "libraries": { + "A": { + "L": "0x2323232232323" + } + }, + "outputSelection": { + "*": { + "C": ["evm.bytecode.object", "evm.bytecode.linkReferences"] + } + } + } +} diff --git a/test/cmdlineTests/linking_standard_solidity_invalid_length_address/output.json b/test/cmdlineTests/linking_standard_solidity_invalid_length_address/output.json new file mode 100644 index 000000000..b3fd6d6cf --- /dev/null +++ b/test/cmdlineTests/linking_standard_solidity_invalid_length_address/output.json @@ -0,0 +1,12 @@ +{ + "errors": + [ + { + "component": "general", + "formattedMessage": "Library address is of invalid length.", + "message": "Library address is of invalid length.", + "severity": "error", + "type": "JSONError" + } + ] +} diff --git a/test/solc/CommandLineParser.cpp b/test/solc/CommandLineParser.cpp index 43cef9d27..b85bd26de 100644 --- a/test/solc/CommandLineParser.cpp +++ b/test/solc/CommandLineParser.cpp @@ -399,6 +399,76 @@ BOOST_AUTO_TEST_CASE(standard_json_mode_options) BOOST_TEST(parsedOptions == expectedOptions); } +BOOST_AUTO_TEST_CASE(invalid_library_address_length) +{ + vector commandLine = { + "solc", + "contract.sol", + "--libraries=" + "dir1/file1.sol:L=0x" + }; + + string expectedMessage = "Invalid length for address for library \"dir1/file1.sol:L\": 0 instead of 40 characters."; + auto hasCorrectMessage = [&](CommandLineValidationError const& _exception) { + return _exception.what() == expectedMessage; + }; + + BOOST_CHECK_EXCEPTION(parseCommandLine(commandLine), CommandLineValidationError, hasCorrectMessage); +} + +BOOST_AUTO_TEST_CASE(invalid_library_address_empty) +{ + vector commandLine = { + "solc", + "contract.sol", + "--libraries=" + "dir1/file1.sol:L=" + }; + + string expectedMessage = "Empty address provided for library \"dir1/file1.sol:L\".\n" + "Note that there should not be any whitespace after the equal sign." ; + auto hasCorrectMessage = [&](CommandLineValidationError const& _exception) { + return _exception.what() == expectedMessage; + }; + + BOOST_CHECK_EXCEPTION(parseCommandLine(commandLine), CommandLineValidationError, hasCorrectMessage); +} + +BOOST_AUTO_TEST_CASE(invalid_library_address_prefix) +{ + vector commandLine = { + "solc", + "contract.sol", + "--libraries=" + "dir1/file1.sol:L=1111122222333334444455555666667777788888" + }; + + string expectedMessage = "The address 1111122222333334444455555666667777788888 is not prefixed with \"0x\".\n" + "Note that the address must be prefixed with \"0x\"." ; + auto hasCorrectMessage = [&](CommandLineValidationError const& _exception) { + return _exception.what() == expectedMessage; + }; + + BOOST_CHECK_EXCEPTION(parseCommandLine(commandLine), CommandLineValidationError, hasCorrectMessage); +} + +BOOST_AUTO_TEST_CASE(invalid_library_address_checksum) +{ + vector commandLine = { + "solc", + "contract.sol", + "--libraries=" + "dir1/file1.sol:L=0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaaa" + }; + + string expectedMessage = "Invalid checksum on address for library \"dir1/file1.sol:L\": aAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaaa\n" + "The correct checksum is 0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaAa"; + auto hasCorrectMessage + = [&](CommandLineValidationError const& _exception) { return _exception.what() == expectedMessage; }; + + BOOST_CHECK_EXCEPTION(parseCommandLine(commandLine), CommandLineValidationError, hasCorrectMessage); +} + BOOST_AUTO_TEST_CASE(invalid_options_input_modes_combinations) { map> invalidOptionInputModeCombinations = { From 01128d6c164bcf126fac7f48cbe049d49b3636ee Mon Sep 17 00:00:00 2001 From: ssi91 Date: Sat, 26 Aug 2023 18:37:31 -0400 Subject: [PATCH 2/3] unify libraries' address verification --- libsolidity/interface/StandardCompiler.cpp | 24 +++++++------ libsolutil/StringUtils.cpp | 24 +++++++++++++ libsolutil/StringUtils.h | 12 +++++++ solc/CommandLineParser.cpp | 39 +++++++++++++--------- 4 files changed, 73 insertions(+), 26 deletions(-) diff --git a/libsolidity/interface/StandardCompiler.cpp b/libsolidity/interface/StandardCompiler.cpp index c46be3b22..03adddc60 100644 --- a/libsolidity/interface/StandardCompiler.cpp +++ b/libsolidity/interface/StandardCompiler.cpp @@ -881,29 +881,31 @@ std::variant StandardCompiler: if (!jsonSourceName[library].isString()) return formatFatalError(Error::Type::JSONError, "Library address must be a string."); std::string address = jsonSourceName[library].asString(); + util::h160 addr; - if (!boost::starts_with(address, "0x")) + util::ValidationError error = util::validateAddress(address, addr); + + switch (error) + { + case util::ValidationError::emptyAddress: + case util::ValidationError::wrongPrefix: return formatFatalError( Error::Type::JSONError, "Library address is not prefixed with \"0x\"." ); - - if (address.length() != 42) + case util::ValidationError::wrongAddressLength: return formatFatalError( Error::Type::JSONError, "Library address is of invalid length." ); - - try - { - ret.libraries[sourceName + ":" + library] = util::h160(address); - } - catch (util::BadHexCharacter const&) - { + case util::ValidationError::checksumNotPassed: + case util::ValidationError::invalidAddress: return formatFatalError( Error::Type::JSONError, - "Invalid library address (\"" + address + "\") supplied." + "Invalid library address (\"0x" + address + "\") supplied." ); + case util::ValidationError::noError: + ret.libraries[sourceName + ":" + library] = addr; } } } diff --git a/libsolutil/StringUtils.cpp b/libsolutil/StringUtils.cpp index 1dc86714d..e9b016612 100644 --- a/libsolutil/StringUtils.cpp +++ b/libsolutil/StringUtils.cpp @@ -22,6 +22,7 @@ * String routines */ +#include "FixedHash.h" #include #include #include @@ -190,3 +191,26 @@ std::string solidity::util::formatNumberReadable(bigint const& _value, bool _use return sign + str; } +ValidationError solidity::util::validateAddress(std::string& addrString, util::h160& address) +{ + if (addrString.empty()) + return ValidationError::emptyAddress; + + if (addrString.substr(0, 2) != "0x") + return ValidationError::wrongPrefix; + addrString = addrString.substr(2); + + if (addrString.length() != 40) + return ValidationError::wrongAddressLength; + + if (!util::passesAddressChecksum(addrString, false)) + return ValidationError::checksumNotPassed; + + bytes binAddr = util::fromHex(addrString); + address = util::h160(binAddr, util::h160::AlignRight); + + if (binAddr.size() > 20 || address == util::h160()) + return ValidationError::invalidAddress; + + return ValidationError::noError; +} diff --git a/libsolutil/StringUtils.h b/libsolutil/StringUtils.h index 0d5bc7839..d45932d7b 100644 --- a/libsolutil/StringUtils.h +++ b/libsolutil/StringUtils.h @@ -26,6 +26,7 @@ #include #include +#include #include @@ -115,6 +116,17 @@ std::string joinHumanReadablePrefixed /// @example formatNumberReadable(-57896044618658097711785492504343953926634992332820282019728792003956564819968) = -2**255 std::string formatNumberReadable(bigint const& _value, bool _useTruncation = false); +enum ValidationError { + noError = 0, + emptyAddress, + wrongPrefix, + wrongAddressLength, + checksumNotPassed, + invalidAddress +}; + +ValidationError validateAddress(std::string& addrString, util::h160& address); + /// Safely converts an unsigned integer as string into an unsigned int type. /// /// @return the converted number or nullopt in case of an failure (including if it would not fit). diff --git a/solc/CommandLineParser.cpp b/solc/CommandLineParser.cpp index 53f481ae8..8cb6ffac3 100644 --- a/solc/CommandLineParser.cpp +++ b/solc/CommandLineParser.cpp @@ -23,6 +23,7 @@ #include #include +#include #include @@ -411,43 +412,51 @@ void CommandLineParser::parseLibraryOption(std::string const& _input) std::string addrString(lib.begin() + static_cast(separator) + 1, lib.end()); boost::trim(addrString); - if (addrString.empty()) + + util::h160 address; + util::ValidationError error = util::validateAddress(addrString, address); + + switch (error) + { + case util::ValidationError::emptyAddress: solThrow( CommandLineValidationError, "Empty address provided for library \"" + libName + "\".\n" - "Note that there should not be any whitespace after the " + + "Note that there should not be any whitespace after the " + (isSeparatorEqualSign ? "equal sign" : "colon") + "." ); - - if (addrString.substr(0, 2) == "0x") - addrString = addrString.substr(2); - else + break; + case util::ValidationError::wrongPrefix: solThrow( CommandLineValidationError, "The address " + addrString + " is not prefixed with \"0x\".\n" - "Note that the address must be prefixed with \"0x\"." + "Note that the address must be prefixed with \"0x\"." ); - - if (addrString.length() != 40) + break; + case util::ValidationError::wrongAddressLength: solThrow( CommandLineValidationError, "Invalid length for address for library \"" + libName + "\": " + std::to_string(addrString.length()) + " instead of 40 characters." ); - if (!util::passesAddressChecksum(addrString, false)) + break; + case util::ValidationError::checksumNotPassed: solThrow( CommandLineValidationError, "Invalid checksum on address for library \"" + libName + "\": " + addrString + "\n" - "The correct checksum is " + util::getChecksummedAddress(addrString) + "The correct checksum is " + util::getChecksummedAddress(addrString) ); - bytes binAddr = util::fromHex(addrString); - util::h160 address(binAddr, util::h160::AlignRight); - if (binAddr.size() > 20 || address == util::h160()) + break; + case util::ValidationError::invalidAddress: solThrow( CommandLineValidationError, "Invalid address for library \"" + libName + "\": " + addrString ); - m_options.linker.libraries[libName] = address; + break; + case util::ValidationError::noError: + m_options.linker.libraries[libName] = address; + break; + } } } From 4aca2abcb3dea0ca74bfaee169273dde575c2518 Mon Sep 17 00:00:00 2001 From: ssi91 Date: Sat, 26 Aug 2023 19:30:58 -0400 Subject: [PATCH 3/3] code-style fixes --- libsolutil/StringUtils.cpp | 2 +- solc/CommandLineParser.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libsolutil/StringUtils.cpp b/libsolutil/StringUtils.cpp index e9b016612..26bb3246f 100644 --- a/libsolutil/StringUtils.cpp +++ b/libsolutil/StringUtils.cpp @@ -22,7 +22,7 @@ * String routines */ -#include "FixedHash.h" +#include #include #include #include diff --git a/solc/CommandLineParser.cpp b/solc/CommandLineParser.cpp index 8cb6ffac3..95419939c 100644 --- a/solc/CommandLineParser.cpp +++ b/solc/CommandLineParser.cpp @@ -430,7 +430,7 @@ void CommandLineParser::parseLibraryOption(std::string const& _input) solThrow( CommandLineValidationError, "The address " + addrString + " is not prefixed with \"0x\".\n" - "Note that the address must be prefixed with \"0x\"." + "Note that the address must be prefixed with \"0x\"." ); break; case util::ValidationError::wrongAddressLength: @@ -444,7 +444,7 @@ void CommandLineParser::parseLibraryOption(std::string const& _input) solThrow( CommandLineValidationError, "Invalid checksum on address for library \"" + libName + "\": " + addrString + "\n" - "The correct checksum is " + util::getChecksummedAddress(addrString) + "The correct checksum is " + util::getChecksummedAddress(addrString) ); break; case util::ValidationError::invalidAddress: