go-safeweb icon indicating copy to clipboard operation
go-safeweb copied to clipboard

bancheck: Does it work for methods of concrete types?

Open kele opened this issue 3 years ago • 5 comments

Example:

package tools

type Fooer struct {}

func (*Fooer) Foo() {}
  1. Would adding tools.(*Fooer).Foo to the banned API list ban calls like f.Foo()?
  2. Do we want to explicitly discourage this? Seems easy to bypass.
package bypassing

func caller() {
  f := &tools.Fooer{}
  f.Foo() // <- currently banned.
}

type FooInterface interface { Foo() }

func bypass() {
  var f tools.FooInterface
  f = &tools.Fooer{}
  f.Foo() // <- there is no way we can ban this
}

kele avatar May 19 '21 07:05 kele

Can we actually prevent this from happening?

Blocking the whole signature, regardless the package it comes from, might have some false positives, don't you think?

empijei avatar May 20 '21 18:05 empijei

empijei@:

Can we actually prevent this from happening? Blocking the whole signature, regardless the package it comes from, might have some false positives, don't you think?

This is what I was pointing at with:

  1. Do we want to explicitly discourage this? Seems easy to bypass.

Either you have something that is trivial to bypass with creating your own interface, or you end up with lots of false positives. Therefore I would completely remove the option of banning methods.

kele avatar May 20 '21 20:05 kele

So if you want to ban a method you have to ban the entire type?

empijei avatar May 21 '21 09:05 empijei

I don't know whether we can ban types (i.e. whether this is technically feasible and non-bypassable trivially).

If one owns the API they wish to ban, they can always provide top-level methods that have access to package-private fields of the type they wish to interact with. This, unfortunately, requires some forethought.

kele avatar May 21 '21 09:05 kele

+1 We should probably discuss this together.

empijei avatar May 21 '21 10:05 empijei