chains-project / GoSurf

Static analyzer to find locations to hide malicious code in Go
2 stars 1 forks source link

reflection: method invocation approach. #5

Open vivi365 opened 4 months ago

vivi365 commented 4 months ago

This is not quite ready.

The blacklisted function approach is not sufficient. It e.g. does not catch m.Call() where m is of type reflect.Value and thus misses to catch the POC case.

I need to look into this a bit more

vivi365 commented 4 months ago

To find calls via identifiers like m we need to type check the file.

v := reflect.ValueOf(f)
m := v.MethodByName("Method"
m.Call(nil)

This is implemented with packages go/importer and go/types.

This parser now catches the POC case!

CAVEATS:

  1. Type checking files: I encountered several errors of type checking files for go ethereum (importing issue). I chose to ignore the errors (could not find easy fix), which means some cases of reflection method invocations could potentially be missed.
// LIMITATION/TODO: not all files can be type checked due to import issues
if err != nil {
    //fmt.Printf("Error type-checking file %s: %v\n", path, err)
}
  1. Reported occurrences: For go-ethereum/accounts, capslock finds 9 occurrences, whereas gosurface finds 19.

I should look into the exact reported cases we get...

  1. Overhead?: I have not checked overhead for introducing type checking, but this was the only viable option I found
vivi365 commented 4 months ago

Regarding point 2. Reported occurrences

As previously known, capslock does not check subdirectories. This is the reason it finds fewer occurrences (also capslock checks transitive uses as well, we do not do this, so it would still probably differ if checking subdirs).

vivi365 commented 4 months ago

Note: should probably have proper tests...