Cyfrin / aderyn

Aderyn 🦜 Rust-based Solidity AST analyzer.
https://docs.cyfrin.io/
GNU General Public License v2.0
416 stars 62 forks source link

Refactor More Detector Names to suit `//aderyn-ignore..` pattern #739

Closed TilakMaddy closed 1 month ago

TilakMaddy commented 2 months ago

Follow ups from https://github.com/Cyfrin/aderyn/pull/737

Please feel free to improve them even more.

Basically what I'm looking for is that when we write //aderyn-ignore(rule-name)

It should be CONSISTENT! We "ignore" the "wrong" thing. Therefore rule-name must specify the WRONG thing.

For example, //aderyn-ignore(require-without-string) instead of //aderyn-ignore(require-with-string)

I get it that we want to have a string as explanation in the require statement. But .. that's where the description/recommendation/hint comes into play.

Tldr; It should read like .... Here's the problem. and we're ignoring it Not like .. Here's the correct thing to do .. and we're ignoring it.

Other examples that would make sense

//aderyn-allow-next-line(require-without-string) //aderyn-allow-next-line(require-without-explanation)

ETC, ETC

@alexroan Please feel free to change it up as you see makes sense

TilakMaddy commented 1 month ago

@alexroan Since we haven't announced the ignore pattern yet, I think we have a lot of room for improvement / flexibility. We need to take action quickly here otherwise once people start using //aderyn-ignore(detector-name) it would be a pain in the neck to change all those to //aderyn-ignore(insert-new-detector-name) ..

Although I'm merging this, please take a careful look once again and propose new changes (if any) before we release 0.3.1

I'll assign this to you in linear as well :)

TilakMaddy commented 1 month ago

New name list here

ArrayLengthNotCached StateChangeInAssert HashCollisionDueToAbiEncodePacked SignatureMalleabilityDueToRawEcrecover NoZeroAddressCheck SendsEtherAwayWithoutCheckingAddress
DelegateCallOnUncheckedAddress RedundantBooleanEquality