search

dgryski / semgrep-go

Go rules for semgrep and go-ruleguard
MIT License
460 stars 37 forks source link

syscall usage #40

Open dnwe opened 3 years ago

dnwe commented 3 years ago

Having encountered some unnecessary syscall usage on a recent PR review, it seems like there's a couple of areas for possible semgrep/ruleguard coverage around the use of the deprecated syscall package

  1. Low-level syscall funcs should be replaced via the cross-platform equivalent os, time, net etc. func calls
  2. If a syscall is truly needed then they should be calling the func in the appropriate non-deprecated golang.org/x/sys pkg instead?
dgryski commented 3 years ago

This sounds great. Do you have a specific example we could add that has a low false-positive rate?

dnwe commented 3 years ago

Simple things like syscall Chmod, Chown and Setenv, Unsetenv, Clearenv mapping to the os versions would be a good start I think. I’m not sure there would ever be a good reason to use those basic ones outside the std library

One interesting conundrum are the various const values in syscall such as S_IRUSR and whether they are always the same between golang.org/x/sys/unix and golang.org/x/sys/windows

dgryski commented 3 years ago

That sounds reasonable. I'll try to write up a rule for some of these.

ainar-g commented 3 years ago

So far, the only “need” I've seen for syscall as opposed to golang.org/x/sys/unix and friends is the syscall.RawConn interface. And it's not really a need, because thanks to the way Go interfaces work, you can redefine a local copy of that interface.

syscall.Errno could be another one, but I'm pretty sure that by now most of these are mapped to os.ErrNotExist and such, and so can be detected using a simple errors.Is.