Skip to content

Fix G118 false positive when cancel is stored in returned struct field#1593

Merged
ccojocar merged 1 commit intosecurego:masterfrom
ccojocar:fix/g118-struct-field-cancel-false-positive
Mar 9, 2026
Merged

Fix G118 false positive when cancel is stored in returned struct field#1593
ccojocar merged 1 commit intosecurego:masterfrom
ccojocar:fix/g118-struct-field-cancel-false-positive

Conversation

@ccojocar
Copy link
Copy Markdown
Member

@ccojocar ccojocar commented Mar 9, 2026

Summary

Fixes #1591

The G118 analyzer (detectLostCancel) reported a false positive when a cancel function was stored in a struct field and the struct was returned to the caller:

type Foo struct{ Cancel func() }

func NewFoo() Foo {
    _, cancel := context.WithCancel(context.Background())
    foo := Foo{Cancel: cancel}
    return foo
}

func main() {
    foo := NewFoo()
    foo.Cancel()
}

Root cause

In isCancelCalled, when the cancel value is stored into a struct field via a Store to a FieldAddr, the trace adds the FieldAddr address to the queue. But that FieldAddr's only referrer is the Store itself — dead end. The struct is subsequently loaded and returned, transferring responsibility to the caller, but this link was never followed.

The existing isCancelCalledViaStructField only checks methods on the same receiver type, which doesn't apply when the struct has no methods and the cancel is consumed by a plain caller.

Fix

Added isStructFieldReturnedFromFunc that checks whether the struct base pointer from a FieldAddr is loaded and returned from the enclosing function. Called from the Store+FieldAddr branch in isCancelCalled — if the struct is returned, treat the cancel as transferred (same as a direct cancel return).

Testing

  • Added a test sample matching the exact pattern from the issue (expected 0 issues)
  • All existing G118 tests continue to pass
  • Verified manually that genuinely lost cancels (struct not returned) are still detected

When a cancel function is stored in a struct field and the struct is
returned to the caller, the cancel responsibility is transferred.
isCancelCalled did not detect this pattern because the FieldAddr trace
reached a dead end — it never connected the field store to the struct
being returned.

Add isStructFieldReturnedFromFunc that checks whether the struct base
pointer of a FieldAddr is loaded and returned, and call it from the
Store+FieldAddr branch in isCancelCalled.

Fixes securego#1591
@ccojocar ccojocar merged commit 0e0eb17 into securego:master Mar 9, 2026
7 checks passed
@ccojocar ccojocar deleted the fix/g118-struct-field-cancel-false-positive branch March 9, 2026 14:49
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 9, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.87%. Comparing base (59a9da0) to head (89a15c0).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
analyzers/context_propagation.go 85.71% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1593   +/-   ##
=======================================
  Coverage   80.87%   80.87%           
=======================================
  Files         104      104           
  Lines        9906     9920   +14     
=======================================
+ Hits         8011     8023   +12     
- Misses       1409     1410    +1     
- Partials      486      487    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

False G118 positive on cancel call on "out of band" call though struct member

1 participant