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.

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

prepare for 0.9.6 release #2030

Merged
merged 16 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/black.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
fetch-depth: 0

- name: Set up Python 3.8
uses: actions/setup-python@v3
uses: actions/setup-python@v4
with:
python-version: 3.8

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- name: Set up Python ${{ matrix.python }}
uses: actions/setup-python@v3
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python }}
- name: Install dependencies
Expand All @@ -67,11 +67,11 @@ jobs:

- name: Set up nix
if: matrix.type == 'dapp'
uses: cachix/install-nix-action@v20
uses: cachix/install-nix-action@v22

- name: Set up cachix
if: matrix.type == 'dapp'
uses: cachix/cachix-action@v10
uses: cachix/cachix-action@v12
with:
name: dapp

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
password: ${{ secrets.GITHUB_TOKEN }}

- name: Docker Build and Push
uses: docker/build-push-action@v3
uses: docker/build-push-action@v4
with:
platforms: linux/amd64,linux/arm64/v8,linux/arm/v7
target: final
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ jobs:
path: './html/'
- name: Deploy to GitHub Pages
id: deployment
uses: actions/deploy-pages@v1
uses: actions/deploy-pages@v2
2 changes: 1 addition & 1 deletion .github/workflows/linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
fetch-depth: 0

- name: Set up Python 3.8
uses: actions/setup-python@v3
uses: actions/setup-python@v4
with:
python-version: 3.8

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
path: dist/

- name: publish
uses: pypa/gh-action-pypi-publish@v1.8.6
uses: pypa/gh-action-pypi-publish@v1.8.7

- name: sign
uses: sigstore/gh-action-sigstore-python@v1.2.3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pylint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
fetch-depth: 0

- name: Set up Python 3.8
uses: actions/setup-python@v3
uses: actions/setup-python@v4
with:
python-version: 3.8

Expand Down
25 changes: 15 additions & 10 deletions slither/detectors/operations/cache_array_length.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from slither.core.declarations import Function
from slither.core.expressions import BinaryOperation, Identifier, MemberAccess, UnaryOperation
from slither.core.solidity_types import ArrayType
from slither.core.source_mapping.source_mapping import SourceMapping
from slither.core.variables import StateVariable
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
from slither.slithir.operations import Length, Delete, HighLevelCall
Expand Down Expand Up @@ -134,7 +133,12 @@ def _is_loop_referencing_array_length(
and op.expression.expression.value == array
):
return True
if isinstance(op, HighLevelCall) and not op.function.view and not op.function.pure:
if (
isinstance(op, HighLevelCall)
and isinstance(op.function, Function)
and not op.function.view
and not op.function.pure
):
return True

for son in node.sons:
Expand All @@ -144,7 +148,7 @@ def _is_loop_referencing_array_length(
return False

@staticmethod
def _handle_loops(nodes: List[Node], non_optimal_array_len_usages: List[SourceMapping]) -> None:
def _handle_loops(nodes: List[Node], non_optimal_array_len_usages: List[Node]) -> None:
"""
For each loop, checks if it has a comparison with `length` array member and, if it has, checks whether that
array size could potentially change in that loop.
Expand Down Expand Up @@ -180,21 +184,21 @@ def _handle_loops(nodes: List[Node], non_optimal_array_len_usages: List[SourceMa
non_optimal_array_len_usages.append(if_node)

@staticmethod
def _get_non_optimal_array_len_usages_for_function(f: Function) -> List[SourceMapping]:
def _get_non_optimal_array_len_usages_for_function(f: Function) -> List[Node]:
"""
Finds non-optimal usages of array length in loop conditions in a given function.
"""
non_optimal_array_len_usages: List[SourceMapping] = []
non_optimal_array_len_usages: List[Node] = []
CacheArrayLength._handle_loops(f.nodes, non_optimal_array_len_usages)

return non_optimal_array_len_usages

@staticmethod
def _get_non_optimal_array_len_usages(functions: List[Function]) -> List[SourceMapping]:
def _get_non_optimal_array_len_usages(functions: List[Function]) -> List[Node]:
"""
Finds non-optimal usages of array length in loop conditions in given functions.
"""
non_optimal_array_len_usages: List[SourceMapping] = []
non_optimal_array_len_usages: List[Node] = []

for f in functions:
non_optimal_array_len_usages += (
Expand All @@ -211,9 +215,10 @@ def _detect(self):
)
for usage in non_optimal_array_len_usages:
info = [
"Loop condition at ",
usage,
" should use cached array length instead of referencing `length` member "
"Loop condition ",
f"`{usage.source_mapping.content}` ",
f"({usage.source_mapping}) ",
"should use cached array length instead of referencing `length` member "
"of the storage array.\n ",
]
res = self.generate_result(info)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
"""
Module detecting unused return values from low level
"""
from slither.detectors.abstract_detector import DetectorClassification
from slither.detectors.operations.unused_return_values import UnusedReturnValues
from typing import List

from slither.core.cfg.node import Node
from slither.slithir.operations import LowLevelCall
from slither.slithir.operations.operation import Operation

from slither.core.declarations.function_contract import FunctionContract
from slither.core.variables.state_variable import StateVariable
from slither.detectors.abstract_detector import (
AbstractDetector,
DetectorClassification,
DETECTOR_INFO,
)
from slither.utils.output import Output


class UncheckedLowLevel(UnusedReturnValues):
class UncheckedLowLevel(AbstractDetector):
"""
If the return value of a send is not checked, it might lead to losing ether
If the return value of a low-level call is not checked, it might lead to losing ether
"""

ARGUMENT = "unchecked-lowlevel"
Expand Down Expand Up @@ -38,5 +47,44 @@ class UncheckedLowLevel(UnusedReturnValues):

WIKI_RECOMMENDATION = "Ensure that the return value of a low-level call is checked or logged."

def _is_instance(self, ir: Operation) -> bool: # pylint: disable=no-self-use
return isinstance(ir, LowLevelCall)
@staticmethod
def detect_unused_return_values(f: FunctionContract) -> List[Node]:
"""
Return the nodes where the return value of a call is unused
Args:
f (Function)
Returns:
list(Node)
"""
values_returned = []
nodes_origin = {}
for n in f.nodes:
for ir in n.irs:
if isinstance(ir, LowLevelCall):
# if a return value is stored in a state variable, it's ok
if ir.lvalue and not isinstance(ir.lvalue, StateVariable):
values_returned.append(ir.lvalue)
nodes_origin[ir.lvalue] = ir

for read in ir.read:
if read in values_returned:
values_returned.remove(read)

return [nodes_origin[value].node for value in values_returned]

def _detect(self) -> List[Output]:
"""Detect low level calls where the success value is not checked"""
results = []
for c in self.compilation_unit.contracts_derived:
for f in c.functions_and_modifiers:
unused_return = UncheckedLowLevel.detect_unused_return_values(f)
if unused_return:

for node in unused_return:
info: DETECTOR_INFO = [f, " ignores return value by ", node, "\n"]

res = self.generate_result(info)

results.append(res)

return results
6 changes: 2 additions & 4 deletions slither/detectors/operations/unused_return_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,8 @@ def detect_unused_return_values(
def _detect(self) -> List[Output]:
"""Detect high level calls which return a value that are never used"""
results = []
for c in self.compilation_unit.contracts:
for f in c.functions + c.modifiers:
if f.contract_declarer != c:
continue
for c in self.compilation_unit.contracts_derived:
for f in c.functions_and_modifiers:
unused_return = self.detect_unused_return_values(f)
if unused_return:

Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
Loop condition at i < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#36) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `j < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#109) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at i_scope_22 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#166) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `i < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#161) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at j_scope_11 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#108) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `i < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#172) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at i_scope_6 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#79) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `j < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#126) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at i_scope_21 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#160) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `k < array2.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#133) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at k_scope_9 < array2.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#98) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `i < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#68) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at k_scope_17 < array2.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#132) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `k < array2.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#99) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at j_scope_15 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#125) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `i < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#167) should use cached array length instead of referencing `length` member of the storage array.

Loop condition at i_scope_4 < array.length (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#67) should use cached array length instead of referencing `length` member of the storage array.
Loop condition `i < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#37) should use cached array length instead of referencing `length` member of the storage array.

Loop condition `i < array.length` (tests/e2e/detectors/test_data/cache-array-length/0.8.17/CacheArrayLength.sol#80) should use cached array length instead of referencing `length` member of the storage array.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ contract CacheArrayLength

S[] array;
S[] array2;
uint public x;

function h() external
{
Expand Down Expand Up @@ -167,5 +168,10 @@ contract CacheArrayLength
{
this.h_view();
}
// array not modified and it cannot be changed in a function call since x is a public state variable
for (uint i = 0; i < array.length; i++) // warning should appear
{
this.x();
}
}
}
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@ contract MyConc{
}

function good(address payable dst) external payable{
(bool ret, bytes memory _) = dst.call{value:msg.value}("");
(bool ret, ) = dst.call{value:msg.value}("");
require(ret);
}

function good2(address payable dst) external payable{
(bool ret, ) = dst.call{value:msg.value}("");
if (!ret) {
revert();
}
}
}
Binary file not shown.