crytic / amarna

Amarna is a static-analyzer and linter for the Cairo programming language.
https://blog.trailofbits.com/2022/04/20/amarna-static-analysis-for-cairo-programs/
GNU Affero General Public License v3.0
148 stars 7 forks source link

Handle function import aliasing to detect function usage #2

Closed fcasal closed 2 years ago

fcasal commented 2 years ago

If functions are imported with an alias from file import function as another_name, they are not handled correctly in the unused functions rule.

coolhill commented 2 years ago

If the imported (with alias) function is not used will it not be covered in the unused-imports rule? Why do we need this change specifically? For example

coolhill@coolhill:~/amarna$ cat import.cairo
from services.exchange.cairo.signature_message_hashes import transfer_hash as exchange_transfer_hash

func transfer_hash() -> ():
    let (transfer_hash) = exchange_transfer_hash()
    return (transfer_hash)
end
coolhill@coolhill:~/amarna$ amarna import.cairo -s

coolhill@coolhill:~/amarna$ sed -i 's/let/#let/g' import.cairo
coolhill@coolhill:~/amarna$ cat import.cairo
from services.exchange.cairo.signature_message_hashes import transfer_hash as exchange_transfer_hash

func transfer_hash() -> ():
    #let (transfer_hash) = exchange_transfer_hash()
    return (transfer_hash)
end
coolhill@coolhill:~/amarna$ amarna import.cairo -s
[unused-imports] in import.cairo:1:79
coolhill@coolhill:~/amarna$ 

Or is there some case I am not considering?

fcasal commented 2 years ago

The issue is for the unused Functions detector. If you have two files like

# library.cairo

func foo() -> ():
end

func bar() -> ():
end

and

# main.cairo
from library import foo as bar

func main() -> ():
    bar()
end

Here, amarna reports that foo is never used, but in fact it is bar that is never used. This detector should probably also use the new import gatherer.