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

Detector: State variable that could be declared constant #672

Merged
merged 52 commits into from
Sep 2, 2024
Merged

Detector: State variable that could be declared constant #672

merged 52 commits into from
Sep 2, 2024

Conversation

TilakMaddy
Copy link
Collaborator

@TilakMaddy TilakMaddy commented Aug 19, 2024 •
edited
Loading

Severity: Low

Qualifications: All solc versions

Logic:

  1. Collect all state variables that are not marked constant or immutable and are also not structs/mappings/contracts (collection A)
  2. Investigate every function and collect all the state variables that could have changed (collection B)
  3. Result = collection A - collection B

TilakMaddy and others added 30 commits August 8, 2024 13:34
@TilakMaddy
finder
eac3d08
@TilakMaddy
has variable been manipulated
859389b
@TilakMaddy
clippy fix
fefc4cf
@TilakMaddy
fixed size array tests
7579a0b
@TilakMaddy
more test coverage
ebe123b
@TilakMaddy
assigning to storage pointer case covered
29ad0ea
@TilakMaddy
finder fix
8a8b579
@TilakMaddy
cli/reportgen
6a3159a 
clippy

cli/reportgen
@TilakMaddy
no struct assignment test
47c8382
@TilakMaddy
dynamic array direct push
8e9dcb9
@TilakMaddy
tests work
c19d7b2
@TilakMaddy
approxiamate state variable manipulation finder
6abae97 
heuristics to link state variables and storage pointers part 1

heuristics to link state variables and storage pointers part 2

heuristics to link state variables and storage pointers part 3
@TilakMaddy
spelling errir
2fd3cf5
@TilakMaddy
more assertions in tests
e7e589d
@TilakMaddy
more public functions to interact
b91098b
@TilakMaddy
impl Add for Finder
e12ae16
@TilakMaddy
docs
a20d5f0
@TilakMaddy
checkpoint
c76cf97 
checkpoint 2

checkpoint 2

checkpoint 3

checkpoint 3

checkpoint 3
@TilakMaddy
yanked read finder. back to only finding writes/changes
7685af6
@TilakMaddy
clippy fix
979d871
@TilakMaddy
merge dev
90a42ee
@TilakMaddy
test for +=, -=, *= (assignment operations)
f5d137d
@TilakMaddy
test for --, ++ (unary operations)
d5e8cf4
@TilakMaddy
cli/reportgen
18c1e44
@TilakMaddy
serial tests
d6665c5
@programskillforverification @TilakMaddy
Feat: add python binding (#640)
bcfe88d
@TilakMaddy @alexroan
Detector: Incorrect ERC721 contract (#655)
8ec370b 
Co-authored-by: Alex Roan <[email protected]>
@TilakMaddy @alexroan
Detector: Builtin Symbol Shadow (#665)
fe6bc28 
Co-authored-by: Alex Roan <[email protected]>
@alexroan @TilakMaddy
Refactor Callgraph (#669)
802490b 
Co-authored-by: TilakMaddy <[email protected]>
@TilakMaddy
state change
bbc09ee
@TilakMaddy TilakMaddy marked this pull request as ready for review August 19, 2024 06:12
@TilakMaddy TilakMaddy marked this pull request as draft August 19, 2024 06:48
@TilakMaddy TilakMaddy marked this pull request as ready for review August 19, 2024 06:58
@TilakMaddy TilakMaddy mentioned this pull request Aug 19, 2024
Closed
@TilakMaddy TilakMaddy added the priority-high High priority Issue - Tackle this first. label Aug 28, 2024
alexroan
alexroan requested changes Aug 28, 2024
View reviewed changes
Copy link
Contributor

@alexroan alexroan left a comment

Choose a reason for hiding this comment

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

To fix:

  1. A false negative
  2. Report noise

aderyn_core/src/detect/low/state_variable_could_be_constant.rs Outdated Show resolved Hide resolved
reports/report.md Outdated Show resolved Hide resolved
tests/contract-playground/src/StateVariablesManipulation.sol Outdated Show resolved Hide resolved
@TilakMaddy TilakMaddy marked this pull request as draft August 29, 2024 07:02
@TilakMaddy
Copy link
Collaborator Author

@alexroan

  1. False negative addressed
  2. Noise removed

@TilakMaddy TilakMaddy marked this pull request as ready for review September 2, 2024 07:06
@alexroan alexroan merged commit f55568b into dev Sep 2, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexroan alexroan alexroan approved these changes

Assignees
No one assigned
Labels
priority-high High priority Issue - Tackle this first. state-variable-change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TilakMaddy @alexroan @programskillforverification