Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault on incomplete threat model #66

Open
CMon opened this issue Apr 16, 2024 · 2 comments
Open

Segmentation fault on incomplete threat model #66

CMon opened this issue Apr 16, 2024 · 2 comments
Assignees

Comments

@CMon
Copy link
Contributor

CMon commented Apr 16, 2024

I create a threat model and am in the middle of creation, now I need to clean out all stuff i forget. During this journey I encounter some crashes (see #65). Now I have one I can not easily fix myself:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x6d818a]

goroutine 1 [running]:
github.com/threagile/threagile/pkg/security/risks/builtin.(*ServerSideRequestForgeryRule).createRisk(0xcbd480?, 0xc0001a4d88, 0xc0004c1760, 0xc000304ea0)
        /app/pkg/security/risks/builtin/server-side-request-forgery-rule.go:92 +0x70a
github.com/threagile/threagile/pkg/security/risks/builtin.(*ServerSideRequestForgeryRule).GenerateRisks(0x160b560, 0xc0001a4d88)
        /app/pkg/security/risks/builtin/server-side-request-forgery-rule.go:56 +0x305
github.com/threagile/threagile/pkg/model.applyRiskGeneration(0xc0001a4d88, 0xc00026a030, {0x160b560, 0x0, 0x8?}, {0xf45468, 0xc0000129fe})
        /app/pkg/model/read.go:97 +0x2a2
github.com/threagile/threagile/pkg/model.ReadAndAnalyzeModel(0xc0004e1188, {0xf45468, 0xc0000129fe})
        /app/pkg/model/read.go:56 +0x539
github.com/threagile/threagile/internal/threagile.(*Threagile).initAnalyze.func1(0xc0004bcc00?, {0xda0c41?, 0x4?, 0xda0c45?})
        /app/internal/threagile/analyze.go:21 +0xec
github.com/spf13/cobra.(*Command).execute(0xc0004ec608, {0xc000467240, 0x4, 0x4})
        /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0xaca
github.com/spf13/cobra.(*Command).ExecuteC(0xc0004ec308)
        /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
        /go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/threagile/threagile/internal/threagile.(*Threagile).Execute(0xc0006bef00)
        /app/internal/threagile/threagile.go:16 +0x25
main.main()
        /app/cmd/threagile/main.go:12 +0x32

I added (in pkg/security/risks/builtin/server-side-request-forgery-rule.go

	// report error if no trustboundary found, this does not fix the crash but at least I know where to fix stuff
	if input.TrustBoundaries[technicalAsset.GetTrustBoundaryId(input)] == nil {
		_, _ = fmt.Fprintf(os.Stderr, "missing trust boundary for technical asset: %q\n", technicalAsset.Id)
	}

before:

	// adjust for cloud-based special risks
	if impact == types.LowImpact && input.TrustBoundaries[technicalAsset.GetTrustBoundaryId(input)].Type.IsWithinCloud() {
		impact = types.MediumImpact
	}

This does not fix the crash but at least I got a hint what I need to fix.

I think the bug is somewhere else, there should be some kind of sanatize method after the parse that checks for the existance of technical assets inside of trust boundaries, and even more if there are more dependencies. or the createRisk methods need a way to report an error.

@joreiche
Copy link
Collaborator

technical assets may or may not be inside trust boundaries so the code should handle it gracefully. a recent change I made changed internal structures to pointers which is why you likely now see crashes where before it just gave you an empty struct. this change was made so that other items can cross-reference items without having to copy them.

I am happy to help fixing the issue you are having. if you need help resolving the issue, please provide a threat model I can use to reproduce the issue

@joreiche joreiche self-assigned this Apr 19, 2024
@CMon
Copy link
Contributor Author

CMon commented Apr 22, 2024

I fixed it inside my threat model by adding the print line, then seeing whats missing and then fixing it in the yaml. But at least in my opinion the threagile tool should already include such error handling. It would be more helpful if such error appear for example in a pipeline than some strange segmentation fault.
I am not very fluent in go, so "seeing" which variables are pointers is not in my skill set. Otherwise I would have suggested to do it the C++ way, check each pointer and report an error if its not valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants