Merge pull request #148 from trailofbits/dev-upgradibility

Add slither-check-upgradability command

.travis.yml

@@ -16,5 +16,6 @@ install:
 script:
   - scripts/travis_test_4.sh
   - scripts/travis_test_5.sh
+  - scripts/travis_test_upgradability.sh
 
 

scripts/travis_install.sh

@@ -8,6 +8,8 @@ function install_solc {
     sudo chmod +x /usr/bin/solc-0.4.25
     sudo wget -O /usr/bin/solc-0.5.1 https://github.com/ethereum/solidity/releases/download/v0.5.1/solc-static-linux
     sudo chmod +x /usr/bin/solc-0.5.1
+    sudo wget -O /usr/bin/solc-0.5.0 https://github.com/ethereum/solidity/releases/download/v0.5.0/solc-static-linux
+    sudo chmod +x /usr/bin/solc-0.5.0
 
     sudo cp /usr/bin/solc-0.5.1 /usr/bin/solc
 }

scripts/travis_test_upgradability.sh

@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+### Test slither-check-upgradability
+
+DIR_TESTS="tests/check-upgradability"
+
+slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 > test_1.txt 2>&1
+DIFF=$(diff test_1.txt "$DIR_TESTS/test_1.txt")
+if [  "$DIFF" != "" ] 
+then
+    echo "slither-check-upgradability failed"
+    cat test_1.txt
+    cat "$DIR_TESTS/test_1.txt"
+    exit -1
+fi
+
+slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2.sol" --new-contract-name ContractV2 > test_2.txt 2>&1 
+DIFF=$(diff test_2.txt "$DIR_TESTS/test_2.txt")
+if [  "$DIFF" != "" ] 
+then
+    echo "slither-check-upgradability failed"
+    cat test_2.txt
+    cat "$DIR_TESTS/test_2.txt"
+    exit -1
+fi
+
+slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug.sol" --new-contract-name ContractV2 > test_3.txt 2>&1 
+DIFF=$(diff test_3.txt "$DIR_TESTS/test_3.txt")
+if [  "$DIFF" != "" ] 
+then
+    echo "slither-check-upgradability failed"
+    cat test_3.txt
+    cat "$DIR_TESTS/test_3.txt"
+    exit -1
+fi
+
+slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug2.sol" --new-contract-name ContractV2 > test_4.txt 2>&1 
+DIFF=$(diff test_4.txt "$DIR_TESTS/test_4.txt")
+if [  "$DIFF" != "" ] 
+then
+    echo "slither-check-upgradability failed"
+    cat test_4.txt
+    cat "$DIR_TESTS/test_4.txt"
+    exit -1
+fi
+
+rm test_1.txt
+rm test_2.txt
+rm test_3.txt
+rm test_4.txt

setup.py

@@ -8,12 +8,13 @@ setup(
     version='0.5.2',
     packages=find_packages(),
     python_requires='>=3.6',
-    install_requires=['prettytable>=0.7.2'],
+    install_requires=['prettytable>=0.7.2', 'pysha3>=1.0.2'],
     license='AGPL-3.0',
     long_description=open('README.md').read(),
     entry_points={
         'console_scripts': [
-            'slither = slither.__main__:main'
+            'slither = slither.__main__:main',
+            'slither-check-upgradability = utils.upgradability.__main__:main'
         ]
     }
 )

slither/__main__.py

@@ -186,7 +186,8 @@ def get_detectors_and_printers():
     from slither.printers.summary.slithir_ssa import PrinterSlithIRSSA
     from slither.printers.summary.human_summary import PrinterHumanSummary
     from slither.printers.functions.cfg import CFG
-
+    from slither.printers.summary.function_ids import FunctionIds
+    from slither.printers.summary.variables_order import VariablesOrder
     printers = [FunctionSummary,
                 ContractSummary,
                 PrinterInheritance,
@@ -196,7 +197,9 @@ def get_detectors_and_printers():
                 PrinterSlithIR,
                 PrinterSlithIRSSA,
                 PrinterHumanSummary,
-                CFG]
+                CFG,
+                FunctionIds,
+                VariablesOrder]
 
     # Handle plugins!
     for entry_point in iter_entry_points(group='slither_analyzer.plugin', name=None):

slither/core/solidity_types/mapping_type.py

@@ -18,7 +18,7 @@ class MappingType(Type):
         return self._to
 
     def __str__(self):
-        return 'mapping({} => {}'.format(str(self._from), str(self._to))
+        return 'mapping({} => {})'.format(str(self._from), str(self._to))
 
     def __eq__(self, other):
         if not isinstance(other, MappingType):

slither/printers/summary/function_ids.py

@@ -0,0 +1,34 @@
+"""
+    Module printing summary of the contract
+"""
+import collections
+from prettytable import PrettyTable
+from slither.printers.abstract_printer import AbstractPrinter
+from slither.utils.colors import blue, green, magenta
+from slither.utils.function import get_function_id
+
+class FunctionIds(AbstractPrinter):
+
+    ARGUMENT = 'function-id'
+    HELP = 'Print the keccack256 signature of the functions'
+
+    def output(self, _filename):
+        """
+            _filename is not used
+            Args:
+                _filename(string)
+        """
+
+        txt = ''
+        for contract in self.slither.contracts_derived:
+            txt += '\n{}:\n'.format(contract.name)
+            table = PrettyTable(['Name', 'ID'])
+            for function in contract.functions:
+                if function.visibility in ['public', 'external']:
+                    table.add_row([function.full_name, hex(get_function_id(function.full_name))])
+            for variable in contract.state_variables:
+                if variable.visibility in ['public']:
+                    table.add_row([variable.name+'()', hex(get_function_id(variable.name+'()'))])
+            txt += str(table) + '\n'
+
+        self.info(txt)

slither/printers/summary/variables_order.py

@@ -0,0 +1,29 @@
+"""
+    Module printing summary of the contract
+"""
+
+from prettytable import PrettyTable
+from slither.printers.abstract_printer import AbstractPrinter
+
+class VariablesOrder(AbstractPrinter):
+
+    ARGUMENT = 'variables-order'
+    HELP = 'Print the storage order of the state variables'
+
+    def output(self, _filename):
+        """
+            _filename is not used
+            Args:
+                _filename(string)
+        """
+
+        txt = ''
+        for contract in self.slither.contracts_derived:
+            txt += '\n{}:\n'.format(contract.name)
+            table = PrettyTable(['Name', 'Type'])
+            for variable in contract.state_variables:
+                if not variable.is_constant:
+                    table.add_row([variable.name, str(variable.type)])
+            txt += str(table) + '\n'
+
+        self.info(txt)

slither/solc_parsing/slitherSolc.py

@@ -3,7 +3,9 @@ import json
 import re
 import logging
 
+logging.basicConfig()
 logger = logging.getLogger("SlitherSolcParsing")
+logger.setLevel(logging.INFO)
 
 from slither.solc_parsing.declarations.contract import ContractSolc04
 from slither.core.slither_core import Slither
@@ -138,6 +140,8 @@ class SlitherSolc(Slither):
 
 
     def _analyze_contracts(self):
+        if not self._contractsNotParsed:
+            logger.info(f'No contract were found in {self.filename}, check the correct compilation') 
         if self._analyzed:
             raise Exception('Contract analysis can be run only once!')
 

slither/utils/function.py

@@ -0,0 +1,14 @@
+import hashlib
+import sha3
+
+def get_function_id(sig):
+    ''''
+        Return the function id of the given signature
+    Args:
+        sig (str)
+    Return:
+        (int)
+    '''
+    s = sha3.keccak_256()
+    s.update(sig.encode('utf-8'))
+    return int("0x" + s.hexdigest()[:8], 16)

tests/check-upgradability/contractV1.sol

@@ -0,0 +1,3 @@
+contract ContractV1{
+    address destination;
+}

tests/check-upgradability/contractV2.sol

@@ -0,0 +1,3 @@
+contract ContractV2{
+    address destination;
+}

tests/check-upgradability/contractV2_bug.sol

@@ -0,0 +1,5 @@
+contract ContractV2{
+    uint destination;
+    
+    uint public myFunc;
+}

tests/check-upgradability/contractV2_bug2.sol

@@ -0,0 +1,6 @@
+contract Base {
+    uint val;
+}
+contract ContractV2 is Base{
+    address destination;
+}

tests/check-upgradability/proxy.sol

@@ -0,0 +1,27 @@
+/*
+   Fake proxy, do not use
+*/
+
+pragma solidity ^0.5.0;
+
+contract Proxy{
+
+    address destination;
+
+    function myFunc() public{}
+
+    function () external{
+        uint size_destination_code;
+        assembly{
+            size_destination_code := extcodesize(destination_slot)
+        }
+        require(size_destination_code>0);
+        (bool ret_status,bytes memory ret_values) = destination.delegatecall(msg.data);
+        require(ret_status);
+        uint length = ret_values.length;
+        assembly{
+            return (ret_values, length)
+        }
+    }
+
+}

tests/check-upgradability/test_1.txt

@@ -0,0 +1,3 @@
+INFO:CompareFunctions:No function id collision found
+INFO:VariablesOrder:Variable in the proxy: destination address
+INFO:VariablesOrder:No error found (variables ordering proxy <-> implementation)

tests/check-upgradability/test_2.txt

@@ -0,0 +1,4 @@
+INFO:CompareFunctions:No function id collision found
+INFO:VariablesOrder:Variable in the proxy: destination address
+INFO:VariablesOrder:No error found (variables ordering proxy <-> implementation)
+INFO:VariablesOrder:No error found (variables ordering between implementations)

tests/check-upgradability/test_3.txt

@@ -0,0 +1,4 @@
+INFO:CompareFunctions:Function id collision found myFunc() myFunc()
+INFO:VariablesOrder:Different variables between proxy and implem: destination address -> destination uint256
+INFO:VariablesOrder:Different variables between v1 and v2: destination address -> destination uint256
+INFO:VariablesOrder:New variable: myFunc uint256

tests/check-upgradability/test_4.txt

@@ -0,0 +1,4 @@
+INFO:CompareFunctions:No function id collision found
+INFO:VariablesOrder:Different variables between proxy and implem: destination address -> val uint256
+INFO:VariablesOrder:Different variables between v1 and v2: destination address -> val uint256
+INFO:VariablesOrder:New variable: destination address

utils/upgradability/__main__.py

@@ -0,0 +1,57 @@
+import logging
+import argparse
+import sys
+
+from slither import Slither
+
+from .compare_variables_order import compare_variables_order_implementation, compare_variables_order_proxy
+from .compare_function_ids import compare_function_ids
+
+logging.basicConfig()
+logger = logging.getLogger("Slither-check-upgradability")
+
+def parse_args():
+
+    parser = argparse.ArgumentParser(description='Slither Upgradability Checks',
+                                     usage="slither-check-upgradability proxy.sol ProxyName implem.sol ContractName")
+
+
+    parser.add_argument('proxy.sol', help='Proxy filename')
+    parser.add_argument('ProxyName', help='Contract name')
+
+    parser.add_argument('implem.sol', help='Implementation filename')
+    parser.add_argument('ContractName', help='Contract name')
+
+    parser.add_argument('--new-version', help='New implementation filename')
+    parser.add_argument('--new-contract-name', help='New contract name (if changed)')
+    parser.add_argument('--solc', help='solc path', default='solc')
+
+    if len(sys.argv) == 1:
+        parser.print_help(sys.stderr)
+        sys.exit(1)
+
+    return parser.parse_args()
+
+def main():
+    args = parse_args()
+
+    proxy = Slither(vars(args)['proxy.sol'], solc=args.solc)
+    proxy_name = args.ProxyName
+    v1 = Slither(vars(args)['implem.sol'], solc=args.solc)
+    v1_name = args.ContractName
+
+    last_contract = v1
+    last_name = v1_name
+
+    if args.new_version:
+        v2 = Slither(args.new_version, solc=args.solc)
+        last_contract = v2
+
+    if args.new_contract_name:
+        last_name = args.new_contract_name
+
+    compare_function_ids(last_contract, proxy)
+    compare_variables_order_proxy(last_contract, last_name, proxy, proxy_name)
+
+    if args.new_version:
+        compare_variables_order_implementation(v1, v1_name, v2, last_name)

utils/upgradability/compare_function_ids.py

@@ -0,0 +1,42 @@
+'''
+    Check for functions collisions between a proxy and the implementation
+    More for information: https://medium.com/nomic-labs-blog/malicious-backdoors-in-ethereum-proxies-62629adf3357
+'''
+
+import logging
+from slither import Slither
+from slither.utils.function import get_function_id
+from slither.utils.colors import red, green
+
+logger = logging.getLogger("CompareFunctions")
+logger.setLevel(logging.INFO)
+
+def get_signatures(s):
+    functions = [contract.functions for contract in s.contracts_derived]
+    functions = [item for sublist in functions for item in sublist]
+    functions = [f.full_name for f in functions if f.visibility in ['public', 'external']]
+
+    variables = [contract.state_variables for contract in s.contracts_derived]
+    variables = [item for sublist in variables for item in sublist]
+    variables = [variable.name+ '()' for variable in variables if variable.visibility in ['public']]
+    return list(set(functions+variables))
+
+
+def compare_function_ids(implem, proxy):
+
+    signatures_implem = get_signatures(implem)
+    signatures_proxy = get_signatures(proxy)
+
+    signatures_ids_implem = {get_function_id(s): s for s in signatures_implem}
+    signatures_ids_proxy = {get_function_id(s): s for s in signatures_proxy}
+
+    found = False
+    for (k, _) in signatures_ids_implem.items():
+        if k in signatures_ids_proxy:
+            found = True
+            logger.info(red('Function id collision found {} {}'.format(signatures_ids_implem[k],
+                                                                       signatures_ids_proxy[k])))
+
+    if not found:
+        logger.info(green('No function id collision found'))
+

utils/upgradability/compare_variables_order.py

@@ -0,0 +1,96 @@
+'''
+    Check if the variables respect the same ordering
+'''
+import logging
+from slither import Slither
+from slither.utils.function import get_function_id
+from slither.utils.colors import red, green, yellow
+
+logger = logging.getLogger("VariablesOrder")
+logger.setLevel(logging.INFO)
+
+def compare_variables_order_implementation(v1, contract_name1, v2, contract_name2):
+
+    contract_v1 = v1.get_contract_from_name(contract_name1)
+    if contract_v1 is None:
+        logger.info(red('Contract {} not found in {}'.format(contract_name1, v1.filename)))
+        exit(-1)
+
+    contract_v2 = v2.get_contract_from_name(contract_name2)
+    if contract_v2 is None:
+        logger.info(red('Contract {} not found in {}'.format(contract_name2, v2.filename)))
+        exit(-1)
+
+
+    order_v1 = [(variable.name, variable.type) for variable in contract_v1.state_variables if not variable.is_constant]
+    order_v2 = [(variable.name, variable.type) for variable in contract_v2.state_variables if not variable.is_constant]
+
+
+    found = False
+    for idx in range(0, len(order_v1)):
+        (v1_name, v1_type) =  order_v1[idx]
+        if len(order_v2) < idx:
+            logger.info(red('Missing variable in the new version: {} {}'.format(v1_name, v1_type)))
+            continue
+        (v2_name, v2_type) =  order_v2[idx]
+
+        if (v1_name != v2_name) or (v1_type != v2_type):
+            found = True
+            logger.info(red('Different variables between v1 and v2: {} {} -> {} {}'.format(v1_name,
+                                                                        v1_type,
+                                                                        v2_name,
+                                                                        v2_type)))
+
+    if len(order_v2) > len(order_v1):
+        new_variables = order_v2[len(order_v1):]
+        for (name, t) in new_variables:
+            logger.info(green('New variable: {} {}'.format(name, t)))
+
+    if not found:
+        logger.info(green('No error found (variables ordering between implementations)'))
+
+def compare_variables_order_proxy(implem, implem_name, proxy, proxy_name):
+
+    contract_implem = implem.get_contract_from_name(implem_name)
+    if contract_implem is None:
+        logger.info(red('Contract {} not found in {}'.format(implem_name, implem.filename)))
+        exit(-1)
+
+    contract_proxy = proxy.get_contract_from_name(proxy_name)
+    if contract_proxy is None:
+        logger.info(red('Contract {} not found in {}'.format(proxy_name, proxy.filename)))
+        exit(-1)
+
+
+    order_implem = [(variable.name, variable.type) for variable in contract_implem.state_variables if not variable.is_constant]
+    order_proxy = [(variable.name, variable.type) for variable in contract_proxy.state_variables if not variable.is_constant]
+
+
+    found = False
+    for idx in range(0, len(order_proxy)):
+        (proxy_name, proxy_type) =  order_proxy[idx]
+        if len(order_proxy) < idx:
+            logger.info(red('Extra variable in the proxy: {} {}'.format(proxy_name, proxy_type)))
+            continue
+        (implem_name, implem_type) =  order_implem[idx]
+
+        if (proxy_name != implem_name) or (proxy_type != implem_type):
+            found = True
+            logger.info(red('Different variables between proxy and implem: {} {} -> {} {}'.format(proxy_name,
+                                                                        proxy_type,
+                                                                        implem_name,
+                                                                        implem_type)))
+        else:
+            logger.info(yellow('Variable in the proxy: {} {}'.format(proxy_name,
+                                                                     proxy_type)))
+
+
+    #if len(order_implem) > len(order_proxy):
+    #    new_variables = order_implem[len(order_proxy):]
+    #    for (name, t) in new_variables:
+    #        logger.info(green('Variable only in implem: {} {}'.format(name, t)))
+
+    if not found:
+        logger.info(green('No error found (variables ordering proxy <-> implementation)'))
+
+