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.

Sign up for GitHub

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

Jump to bottom

FileScope: apply renaming in get_contract_from_name() #1539

Closed
wants to merge 2 commits into from
Closed

FileScope: apply renaming in get_contract_from_name() #1539

wants to merge 2 commits into from

Conversation

SheldonHolmgren
Copy link
Contributor

@SheldonHolmgren SheldonHolmgren commented Dec 31, 2022 •
edited
Loading

This fixes a bug that was making slither crash for https://github.com/makerdao/dss.

Before this patch:

makerdao/dss $ slither . --dapp-ignore-compile
Function not found suck
Impossible to generate IR for PublicClip.kick:
 'NoneType' object has no attribute 'type'
Function not found suck
Impossible to generate IR for PublicClip.redo:
 'NoneType' object has no attribute 'type'
Function not found flux
Impossible to generate IR for PublicClip.take:
 'NoneType' object has no attribute 'type'
Function not found flux
Impossible to generate IR for PublicClip.yank:
 'NoneType' object has no attribute 'type'
Function not found nope
Impossible to generate IR for Cat_.file:
 'NoneType' object has no attribute 'type'
Function not found ilks
Impossible to generate IR for Cat_.bite:
 'NoneType' object has no attribute 'type'
Function not found hope
Impossible to generate IR for TestVow.constructor:
 'NoneType' object has no attribute 'type'
Function not found nope
Impossible to generate IR for TestVow.file:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.heal:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.kiss:
 'NoneType' object has no attribute 'type'
Function not found sin
Impossible to generate IR for TestVow.flop:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.flap:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.cage:
 'NoneType' object has no attribute 'type'
Function not found sin
Impossible to generate IR for TestVow.Awe:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.Joy:
 'NoneType' object has no attribute 'type'
Traceback (most recent call last):
  File "/home/holmgren/dev/slither/slither/__main__.py", line 867, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/holmgren/dev/slither/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/holmgren/dev/slither/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/holmgren/dev/slither/slither/slither.py", line 138, in __init__
    parser.analyze_contracts()
  File "/home/holmgren/dev/slither/slither/solc_parsing/slither_compilation_unit_solc.py", line 500, in analyze_contracts
    self._convert_to_slithir()
  File "/home/holmgren/dev/slither/slither/solc_parsing/slither_compilation_unit_solc.py", line 685, in _convert_to_slithir
    func.generate_slithir_and_analyze()
  File "/home/holmgren/dev/slither/slither/core/declarations/function.py", line 1747, in generate_slithir_and_analyze
    node.slithir_generation()
  File "/home/holmgren/dev/slither/slither/core/cfg/node.py", line 719, in slithir_generation
    self._irs = convert_expression(expression, self)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 118, in convert_expression
    result = apply_ir_heuristics(result, node)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 1787, in apply_ir_heuristics
    irs = propagate_type_and_convert_call(irs, node)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 437, in propagate_type_and_convert_call
    new_ins = propagate_types(ins, node)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 742, in propagate_types
    ir.lvalue.set_type(UserDefinedType(contract))
  File "/home/holmgren/dev/slither/slither/core/solidity_types/user_defined_type.py", line 19, in __init__
    assert isinstance(t, (Contract, Enum, Structure))
AssertionError
Error in .
Traceback (most recent call last):
  File "/home/holmgren/dev/slither/slither/__main__.py", line 867, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/holmgren/dev/slither/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/holmgren/dev/slither/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/holmgren/dev/slither/slither/slither.py", line 138, in __init__
    parser.analyze_contracts()
  File "/home/holmgren/dev/slither/slither/solc_parsing/slither_compilation_unit_solc.py", line 500, in analyze_contracts
    self._convert_to_slithir()
  File "/home/holmgren/dev/slither/slither/solc_parsing/slither_compilation_unit_solc.py", line 685, in _convert_to_slithir
    func.generate_slithir_and_analyze()
  File "/home/holmgren/dev/slither/slither/core/declarations/function.py", line 1747, in generate_slithir_and_analyze
    node.slithir_generation()
  File "/home/holmgren/dev/slither/slither/core/cfg/node.py", line 719, in slithir_generation
    self._irs = convert_expression(expression, self)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 118, in convert_expression
    result = apply_ir_heuristics(result, node)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 1787, in apply_ir_heuristics
    irs = propagate_type_and_convert_call(irs, node)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 437, in propagate_type_and_convert_call
    new_ins = propagate_types(ins, node)
  File "/home/holmgren/dev/slither/slither/slithir/convert.py", line 742, in propagate_types
    ir.lvalue.set_type(UserDefinedType(contract))
  File "/home/holmgren/dev/slither/slither/core/solidity_types/user_defined_type.py", line 19, in __init__
    assert isinstance(t, (Contract, Enum, Structure))
AssertionError

After this patch:

makerdao/dss $ slither . --dapp-ignore-compile
Function not found suck
Impossible to generate IR for PublicClip.kick:
 'NoneType' object has no attribute 'type'
Function not found suck
Impossible to generate IR for PublicClip.redo:
 'NoneType' object has no attribute 'type'
Function not found flux
Impossible to generate IR for PublicClip.take:
 'NoneType' object has no attribute 'type'
Function not found flux
Impossible to generate IR for PublicClip.yank:
 'NoneType' object has no attribute 'type'
Function not found nope
Impossible to generate IR for Cat_.file:
 'NoneType' object has no attribute 'type'
Function not found ilks
Impossible to generate IR for Cat_.bite:
 'NoneType' object has no attribute 'type'
Function not found hope
Impossible to generate IR for TestVow.constructor:
 'NoneType' object has no attribute 'type'
Function not found nope
Impossible to generate IR for TestVow.file:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.heal:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.kiss:
 'NoneType' object has no attribute 'type'
Function not found sin
Impossible to generate IR for TestVow.flop:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.flap:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.cage:
 'NoneType' object has no attribute 'type'
Function not found sin
Impossible to generate IR for TestVow.Awe:
 'NoneType' object has no attribute 'type'
Function not found dai
Impossible to generate IR for TestVow.Joy:
 'NoneType' object has no attribute 'type'

TokenUser.doTransferFrom(address,address,uint256) (test/dai.t.sol#33-38) uses arbitrary from in transferFrom: token.transferFrom(from,to,amount) (test/dai.t.sol#37)
DaiTest.testFailWrongAccountTransfers() (test/dai.t.sol#150-153) uses arbitrary from in transferFrom: token.transferFrom(user2,self,sentAmount) (test/dai.t.sol#152)
DaiTest.testFailTransferWithoutApproval() (test/dai.t.sol#177-180) uses arbitrary from in transferFrom: token.transferFrom(user1,self,1) (test/dai.t.sol#179)
DaiTest.testFailChargeMoreThanApproved() (test/dai.t.sol#182-186) uses arbitrary from in transferFrom: token.transferFrom(user1,self,21) (test/dai.t.sol#185)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#arbitrary-from-in-transferfrom

Pot._rpow(uint256,uint256,uint256) (pot.sol#85-107) uses a weak PRNG: "n % 2 (pot.sol#97)" 
Pot._rpow(uint256,uint256,uint256) (pot.sol#85-107) uses a weak PRNG: "switch_expr_2894_56_19__rpow_asm_0 = n % 2 (pot.sol#89)" 
Jug._rpow(uint256,uint256,uint256) (jug.sol#62-84) uses a weak PRNG: "n % 2 (jug.sol#74)" 
Jug._rpow(uint256,uint256,uint256) (jug.sol#62-84) uses a weak PRNG: "switch_expr_2303_53_18__rpow_asm_0 = n % 2 (jug.sol#66)" 
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#weak-PRNG

(a lot more normal Slither findings follow)

The original crash was happening because import renaming in https://github.com/makerdao/dss/blob/master/src/test/vow.t.sol#L22 was being ignored.

FTR, somewhat related bugs are #1452 and #1364

@0xalpharush 0xalpharush mentioned this pull request Jan 27, 2023
Closed
@0xalpharush 0xalpharush mentioned this pull request Mar 21, 2023
Closed
@0xalpharush 0xalpharush mentioned this pull request Apr 10, 2023
Closed
0xalpharush
0xalpharush reviewed May 15, 2023
View reviewed changes
slither/core/scope/scope.py
Comment on lines +98 to 101
name = name.name
if name in self.renaming:
name = self.renaming[name]
return self.contracts.get(name, None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should only fallback to the renaming if the contract name is not found to minimize potential collisions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xalpharush 0xalpharush 0xalpharush left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SheldonHolmgren @0xalpharush