foundry icon indicating copy to clipboard operation
foundry copied to clipboard

forge audit to check/list all ffi usage

Open OliverNChalk opened this issue 2 years ago • 7 comments

Component

Forge

Describe the feature you would like

Prompted by #2326, it would be good if there was a way to sanity check the ffi usage within a given project. @mds1 pointed out this could be achieved via grep -r \\.ffi. A forge audit command could do something similar but leverage remappings to determine where to search for ffi usage.

Would be good to discuss scope & possible solution space.

Additional context

No response

OliverNChalk avatar Jul 18 '22 04:07 OliverNChalk

Some ideas on what forge audit should include below. IMO the scope of this command should be anything that can result in malicious software on your machine, which means this excludes contract bugs (e.g. we wouldn't run slither or forge lint here).

I'm imagining each item below is its own section of the output that gets printed to the terminal. Probably should support a --json formatted output too.

  1. Potentially dangerous cheat codes. This section would report all usage of cheat codes that touch the filesystem. Right now this is ffi, readFile, readLine, writeFile, writeLine, closeFile, removeFile. One implementation approach is to simply alias this check to printing results from grep, but that assumes grep is installed which wouldn't work on windows so we may need another approach (or a conditional approach based on what tools are installed).
  2. Known vulnerable solidity dependencies. For everything in the lib/ folder, check for known vulnerabilities (analogous to yarn audit and cargo audit, but for contracts). I'm not too familiar with the best way to do this, but I'm guessing the easiest is to just check for security advisories on the specified repo, e.g. see how OZ has a few listed.
  3. Known vulnerable node dependencies. Lots of people have a package.json in their foundry projects and install deps that way. We can check the lockfile to see if it's yarn or npm, and alias this check to the npm audit or yarn audit equivalent.
  4. Known vulnerable rust dependencies. Same as above, but for cargo audit if a Cargo.lock file is present.

May be worth leaving out the last 2 for simplicity, since user's can run those commands manually when applicable.


Using audit as the command name is consistent with similar tools in other ecosystems, but it's also worth considering a different name than audit, since most people will associate that with a security review from an auditing firm. Alternative ideas include forge check, forge report, forge review. I think keeping audit is ok though since it's a standard name for this type of command.

mds1 avatar Jul 18 '22 13:07 mds1

how about forge geiger that lists all unsafe calls?

mattsse avatar Jul 18 '22 17:07 mattsse

Bump

PatrickAlphaC avatar Aug 03 '22 15:08 PatrickAlphaC

@rkrasiuk I think scanning the AST for certain calls would be rather straightforward, right?

mattsse avatar Aug 03 '22 15:08 mattsse

yes, sir. ffi call specifically should roughly look like

FunctionCall(
  _,
  MemberAccess(
    _, Variable(Identifier { loc: _, name: "Vm" }), Identifier { loc: _, name: "ffi" }
  ),
  [ /* args */ ]
)

rkrasiuk avatar Aug 03 '22 16:08 rkrasiuk

with the existing visitors, this should be very easy to write a scanner for calls, right? we just need a list of calls and some matching code?

mattsse avatar Aug 03 '22 16:08 mattsse

yup, you can write the matched functions & their location into some output buffer and that should be it

rkrasiuk avatar Aug 03 '22 17:08 rkrasiuk