Skip to content

Commit

Permalink
Allow non-structs to be source types (#264)
Browse files Browse the repository at this point in the history
Specifically:
- Remove the check that prevents non-structs from being identified as sources
- Rework the logic that determines whether to traverse to the return values of a method call to allow method calls on interfaces to propagate taint from arguments.
  • Loading branch information
mlevesquedion authored Jan 29, 2021
1 parent be2b7a6 commit fb35469
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 13 deletions.
4 changes: 4 additions & 0 deletions internal/pkg/levee/testdata/src/example.com/core/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,7 @@ func (i Innocuous) GetID() int {
func (i Innocuous) GetData() string {
return i.Data
}

type SourceManipulator interface {
Propagate(string) string
}
29 changes: 29 additions & 0 deletions internal/pkg/levee/testdata/src/example.com/tests/method/tests.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package method

import (
"example.com/core"
)

func TestMethodCallOnStaticallyKnownReceiverPropagatesTaint(s core.Source) {
data := s.Propagate(s.Data)
core.Sink(data) // want "a source has reached a sink"
}

func TestMethodCallOnStaticallyUnknownReceiverPropagatesTaint(sm core.SourceManipulator, s core.Source) {
data := sm.Propagate(s.Data)
core.Sink(data) // want "a source has reached a sink"
}
2 changes: 2 additions & 0 deletions internal/pkg/levee/testdata/test-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Sources:
- PackageRE: "^example.com/core$"
TypeRE: "^Source$"
FieldRE: "^Data"
- PackageRE: "^example.com/core$"
TypeRE: "^SourceManipulator$"
Sinks:
- PackageRE: "^example.com/core$"
MethodRE: Sinkf?$
Expand Down
16 changes: 9 additions & 7 deletions internal/pkg/propagation/propagation.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,16 +257,18 @@ func (prop *Propagation) visitCall(call *ssa.Call, maxInstrReached map[*ssa.Basi
// Do traverse if the call is being reached through a tainted argument.
// Source methods that return tainted values regardless of their arguments should be identified by the fieldpropagator analyzer.
if recv := call.Call.Signature().Recv(); recv != nil && sourcetype.IsSourceType(prop.config, prop.taggedFields, recv.Type()) {
visitingFromArg := false
// Interface types cannot be sources. If the receiver is not a source, the
// above condition will be false, so this code won't be executed.
// When the receiver's type is statically known (it isn't an interface type),
// it will be the first element of the Args slice.
// If the receiver is not statically known (it has interface type) and the
// method has no arguments, Args will be empty.
if len(call.Call.Args) == 0 {
fmt.Printf("call.Call.Args was empty; please report this issue")
return
}
for _, a := range call.Call.Args[1:] {
visitingFromArg := false
for i, a := range call.Call.Args {
// If the receiver's type is statically known,
// it will be the first element of the Args slice.
if !call.Call.IsInvoke() && i == 0 {
continue
}
if prop.tainted[a.(ssa.Node)] {
visitingFromArg = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,5 @@ func TestTaggedSourceIdentification() {
_ = TaggedSource{} // want "source identified"
}

// No source should be identified here, because SourceInterface is an interface type.
func TestNamedInterfaceIsNotSource(x SourceInterface) {
func TestNamedInterface(x SourceInterface) { // want "source identified"
}
4 changes: 0 additions & 4 deletions internal/pkg/sourcetype/sourcetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ func IsSourceType(c *config.Config, tf fieldtags.ResultType, t types.Type) bool
deref := utils.Dereference(t)
switch tt := deref.(type) {
case *types.Named:
if _, ok := utils.Dereference(tt.Underlying()).(*types.Interface); ok {
// interfaces cannot be sources
return false
}
return c.IsSourceType(utils.DecomposeType(tt)) || IsSourceType(c, tf, tt.Underlying())
case *types.Array:
return IsSourceType(c, tf, tt.Elem())
Expand Down

0 comments on commit fb35469

Please sign in to comment.