fix(ngap): add nil guards for missing mandatory NGAP-ID IEs in handlers

[donivtech]

Description

Several NGAP handlers dereference a mandatory NGAP-ID IE's .Value right after the IE-decoding loop. The in-loop checks only catch the "IE present but value is nil" case — an IE that is entirely absent from the ProtocolIEs list leaves the pointer nil, so a malformed/non-conformant peer can panic the AMF by omitting it.

The panic occurs in two places per handler: the lookup RanUeFind…(id.Value), and the No UE Context[…: %d] error log immediately after it (which re-derefs the same ID and is reached precisely when the lookup returns nil).

This adds a post-loop if <id> == nil { return } guard before the first dereference in each affected handler.

Note: simply swapping the lookup for the findRanUeBy{Ran,Amf}NgapID helper (as done in #732 for FetchRanUeContext) is not sufficient for the handlers — the helper returns nil, but the subsequent No UE Context log still derefs the ID and panics. The guard is needed because it returns before both the lookup and the log. (FetchRanUeContext is different: it returns the possibly-nil ranUe without logging the ID, so the helper is correct there.)

Handlers guarded (10)

Handler	ID
HandleInitialUEMessage (also return after the criticality-diagnostics ErrorIndication)	RANUENGAPID
HandleInitialContextSetupResponse	RANUENGAPID
HandleInitialContextSetupFailure	RANUENGAPID
HandleUEContextReleaseRequest (nil-safe dual AMFUENGAPID/RANUENGAPID lookup + drop nil-deref from its log)	both
HandleUplinkNasTransport	RANUENGAPID
HandleUEContextReleaseComplete	AMFUENGAPID
HandlePDUSessionResourceReleaseResponse	RANUENGAPID
HandleHandoverCancel	RANUENGAPID
HandleNasNonDeliveryIndication	RANUENGAPID
HandleUERadioCapabilityInfoIndication	RANUENGAPID

The remaining handlers already guard their mandatory IDs (post-loop == nil guard, inline if id != nil wrapper, or the criticality-diagnostics if len > 0 { return } pattern), so they're left unchanged.

Testing

• go build ./..., go vet ./..., go test ./... all pass
• golangci-lint run --new-from-rev=main → 0 issues.
• gofmt clean

[Copilot]

Pull request overview

This PR hardens several NGAP handlers against malformed/non-conformant peers by preventing nil-pointer dereferences when mandatory IEs are absent from ProtocolIEs, avoiding AMF panics/crashes during message handling.

Changes:

• HandleInitialUEMessage: return immediately after sending ErrorIndication for reject/missing IEs; add explicit guard for missing RANUENGAPID before UE lookup/creation.
• HandleInitialContextSetupResponse / HandleInitialContextSetupFailure: add explicit guard for missing RANUENGAPID before UE lookup.
• HandleUEContextReleaseRequest: make AMF/RAN UE ID lookup nil-safe and remove a nil-deref from the “no context” error log path.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

[donivtech]

Good catch. Fixed in c689af4. Added a post-loop nASPDU == nil guard in HandleUplinkNasTransport, HandleInitialUEMessage, and HandleNasNonDeliveryIndication. I also swept all the handlers touched in this PR for any other post-loop IE dereference and found none remaining.

[gab-arrobo]

@donivtech, thanks for your contribution. Please see a few comments below but those comments are applicable across the proposed changes. Let me know what you think. Thanks!

[donivtech]

Switched these to the helpers as suggested (44fe599): findRanUeByRanNgapID / findRanUeByAmfNgapID for the lookups in UplinkNasTransport, UEContextReleaseComplete, PDUSessionResourceReleaseResponse, InitialContextSetupResponse/Failure, HandoverCancel, NasNonDeliveryIndication, and UERadioCapabilityInfoIndication.

In each of these the No UE Context[…: %d] log right after the lookup dereferences the same ID, and with the helper that line is now reached when the ID is nil — so I wrapped it in if id != nil to stay panic-safe (the helper already logs the nil-ID case).

Happy to instead expand the helpers to take a caller label (your "from X" idea) if you'd prefer that over the generic nil-ID log - just say

[gab-arrobo]

Switched these to the helpers as suggested (44fe599): findRanUeByRanNgapID / findRanUeByAmfNgapID for the lookups in UplinkNasTransport, UEContextReleaseComplete, PDUSessionResourceReleaseResponse, InitialContextSetupResponse/Failure, HandoverCancel, NasNonDeliveryIndication, and UERadioCapabilityInfoIndication.

In each of these the No UE Context[…: %d] log right after the lookup dereferences the same ID, and with the helper that line is now reached when the ID is nil — so I wrapped it in if id != nil to stay panic-safe (the helper already logs the nil-ID case).

Happy to instead expand the helpers to take a caller label (your "from X" idea) if you'd prefer that over the generic nil-ID log - just say

Hi @donivtech,
Sorry, I got confused why you needed to check rANUENGAPID == nil. based on your latest changes, I see that you still need to check that because of the log inside if ranUe == nil.

	if rANUENGAPID == nil {
		ran.Log.Errorln("RANUENGAPID IE missing from UplinkNasTransport")
		return
	}

I think a better approach would be to keep the following check(s) and remove the if statements from inside if ranUe == nil. What do you think?

	if rANUENGAPID == nil {
		ran.Log.Errorln("RANUENGAPID IE missing from UplinkNasTransport")
		return
	}

That is, something like this in all places where rANUENGAPID.Value is used inside a if ranUe == nil

diff --git a/ngap/handler.go b/ngap/handler.go
index ed83f62..4b7599e 100644
--- a/ngap/handler.go
+++ b/ngap/handler.go
@@ -789,6 +789,11 @@ func HandleUplinkNasTransport(ctx ctxt.Context, ran *context.AmfRan, message *ng
                }
        }

+       if rANUENGAPID == nil {
+               ran.Log.Errorln("RANUENGAPID IE missing from UplinkNasTransport")
+               return
+       }
+
        if aMFUENGAPID == nil {
                ran.Log.Errorln("AMFUENGAPID IE missing from UplinkNasTransport")
                return
@@ -800,9 +805,7 @@ func HandleUplinkNasTransport(ctx ctxt.Context, ran *context.AmfRan, message *ng

        ranUe := findRanUeByRanNgapID(ran, rANUENGAPID)
        if ranUe == nil {
-               if rANUENGAPID != nil {
-                       ran.Log.Errorf("No UE Context[RanUeNgapID: %d]", rANUENGAPID.Value)
-               }
+               ran.Log.Errorf("No UE Context[RanUeNgapID: %d]", rANUENGAPID.Value)
                return
        }

Thanks!

[donivtech]

Yes, that is better— applied your suggestion in the latest push. Kept the explicit if id == nil { … return } guard before each lookup and removed the if id != nil wrapper from inside if ranUe == nil, while keeping the findRanUeBy*NgapID helper for the lookup itself. So it reads guard → helper → clean No UE Context[…: %d] log (the guard guarantees the ID is non-nil by that point). Also merged latest main in.

Applied across all eight: UplinkNasTransport, UEContextReleaseComplete, PDUSessionResourceReleaseResponse, InitialContextSetupResponse/Failure, HandoverCancel, NasNonDeliveryIndication, UERadioCapabilityInfoIndication.

[donivtech]

Good point , done. HandleUEContextReleaseRequest now uses findRanUeByAmfNgapID(ran, aMFUENGAPID) for the primary lookup (and findRanUeByRanNgapID(ran, rANUENGAPID) for the RAN-ID fallback), dropping the != nil wrappers since the helpers nil-check internally.