guacsec / guac

GUAC aggregates software security metadata into a high fidelity graph database.
https://guac.sh
Apache License 2.0
1.29k stars 176 forks source link

Stackoverflow in recursive call - Parser Cyclonedx #329

Closed naveensrinivasan closed 1 year ago

naveensrinivasan commented 1 year ago

https://github.com/guacsec/guac/blob/e51a961b539b93ab70d6eee27ee0e0876aacb73f/pkg/ingestor/parser/cyclonedx/parser_cyclonedx.go#L57-L67

Here is a test case that will

func Test_addEdges(t *testing.T) {
    packageA := component{curPackage: assembler.PackageNode{Name: "A"}}
    packageB := component{curPackage: assembler.PackageNode{Name: "B"}}
    packageC := component{curPackage: assembler.PackageNode{Name: "C"}}
    packageD := component{curPackage: assembler.PackageNode{Name: "D"}}

    packageA.depPackages = []*component{&packageB}
    packageB.depPackages = []*component{&packageC}
    packageC.depPackages = []*component{&packageD}
    packageD.depPackages = []*component{&packageA}

    var edges []assembler.GuacEdge
    addEdges(packageA, &edges)
}

This test case creates four packages: A, B, C, and D. It sets up a cycle in the dependencies such that A depends on B, B depends on C, C depends on D, and D depends on A. Calling addEdges(packageA, &edges) will cause the function to recursively call itself indefinitely, leading to a stack overflow.

=== RUN   Test_addEdges
runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0x14020360660 stack=[0x14020360000, 0x14040360000]
fatal error: stack overflow
naveensrinivasan commented 1 year ago

This can be fixed by adding a check to see if a package has already been visited before calling the function recursively. One way to do this is to keep track of visited packages in a map and check if the current package is already in the set before calling the function recursively.

We can also add a maximum recursion depth parameter and check if it has reached max depth before calling the function recursively.

I am going to work on resolving this issue.

pxp928 commented 1 year ago

@naveensrinivasan good find!