Skip to content

Commit bfd4ef9

Browse files
donivtechVinod Patmanathanclaude
authored
fix(ngap): add nil guards for three remaining NGAP handlers (#708)
* fix(ngap): add nil guards for three remaining NGAP handlers Follow-up to PR #666. Three handlers still dereference mandatory IE pointers (rANUENGAPID, aMFUENGAPID) without a post-loop nil guard, so a malformed NGAP message that omits the IE entirely from ProtocolIEs.List crashes the AMF. The in-loop checks only catch "IE present but value nil" — they log and continue instead of returning, leaving the variable nil for the post-loop dereference. Handlers fixed: - HandlePDUSessionResourceNotify: guard rANUENGAPID, aMFUENGAPID - HandleHandoverFailure: guard aMFUENGAPID - HandleUplinkRanStatusTransfer: guard rANUENGAPID FetchRanUeContext dispatches PDUSessionResourceNotify and UplinkRANStatusTransfer but does not nil-check aMFUENGAPID for the former and does not parse IEs at all for the latter (empty case). HandoverFailure arrives in UnsuccessfulOutcome which the dispatcher does not pre-process. Signed-off-by: Vinod Patmanathan <vinod.patmanathan@forsway.com> * fix(ngap): address Copilot review on PR #708 - HandlePDUSessionResourceNotify: also guard pDUSessionResourceNotifyList before iterating its .List field. Without the guard, a message that omits this mandatory IE (or includes it with a nil value) still panics on the post-loop range. - HandleHandoverFailure: synthesize a default Cause when the IE is missing. Cause is later dereferenced via *cause when calling SendHandoverPreparationFailure; falling back to the same default (causePresent / causeValue) already used for the SendUEContextRelease path keeps the function crash-safe. - Add tests for the three handlers, matching the style of the existing TestHandleHandoverNotifyIgnoresMissingIDs / TestHandleHandoverRequestAcknowledgeIgnoresMissingAmfUeNgapID. Signed-off-by: Vinod Patmanathan <vinod.patmanathan@forsway.com> * test(ngap): fix HandoverFailure outcome symbols for ngap/v2 v2.1.0 The TestHandleHandoverFailureIgnoresMissingIEs test referenced the ngap/v2 v2.0.0 names for the HandoverFailure unsuccessful outcome, which broke compilation after the dependency was bumped to v2.1.0 (via #711). In v2.1.0 the regenerated ngapType package renamed the outcome to HandoverResourceAllocation: - UnsuccessfulOutcomePresentHandoverFailure -> UnsuccessfulOutcomePresentHandoverResourceAllocation - UnsuccessfulOutcomeValue.HandoverFailure -> UnsuccessfulOutcomeValue.HandoverResourceAllocation The *HandoverFailure struct type itself is unchanged. This fixes the lint, staticcheck, and unit-tests CI failures on PR #708. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Vinod Patmanathan <vinod.patmanathan@forsway.com> --------- Signed-off-by: Vinod Patmanathan <vinod.patmanathan@forsway.com> Co-authored-by: Vinod Patmanathan <vinod.patmanathan@forsway.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 64aa74e commit bfd4ef9

2 files changed

Lines changed: 97 additions & 0 deletions

File tree

‎ngap/handler.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2103,6 +2103,19 @@ func HandlePDUSessionResourceNotify(ctx ctxt.Context, ran *context.AmfRan, messa
21032103
}
21042104
}
21052105

2106+
if rANUENGAPID == nil {
2107+
ran.Log.Errorln("RANUENGAPID IE missing from PDUSessionResourceNotify")
2108+
return
2109+
}
2110+
if aMFUENGAPID == nil {
2111+
ran.Log.Errorln("AMFUENGAPID IE missing from PDUSessionResourceNotify")
2112+
return
2113+
}
2114+
if pDUSessionResourceNotifyList == nil {
2115+
ran.Log.Errorln("PDUSessionResourceNotifyList IE missing from PDUSessionResourceNotify")
2116+
return
2117+
}
2118+
21062119
ranUe = ran.RanUeFindByRanUeNgapID(rANUENGAPID.Value)
21072120
if ranUe == nil {
21082121
ran.Log.Warnf("No UE Context[RanUeNgapID: %d]", rANUENGAPID.Value)
@@ -3611,12 +3624,25 @@ func HandleHandoverFailure(ctx ctxt.Context, ran *context.AmfRan, message *ngapT
36113624
causeValue := ngapType.CauseRadioNetworkPresentHoFailureInTarget5GCNgranNodeOrTargetSystem
36123625
if cause != nil {
36133626
causePresent, causeValue = printAndGetCause(ran, cause)
3627+
} else {
3628+
ran.Log.Warnln("Cause IE missing from HandoverFailure; using default")
3629+
cause = &ngapType.Cause{
3630+
Present: causePresent,
3631+
RadioNetwork: &ngapType.CauseRadioNetwork{
3632+
Value: causeValue,
3633+
},
3634+
}
36143635
}
36153636

36163637
if criticalityDiagnostics != nil {
36173638
printCriticalityDiagnostics(ran, criticalityDiagnostics)
36183639
}
36193640

3641+
if aMFUENGAPID == nil {
3642+
ran.Log.Errorln("AMFUENGAPID IE missing from HandoverFailure")
3643+
return
3644+
}
3645+
36203646
targetUe = context.AMF_Self().RanUeFindByAmfUeNgapID(aMFUENGAPID.Value)
36213647

36223648
if targetUe == nil {
@@ -4040,6 +4066,11 @@ func HandleUplinkRanStatusTransfer(ran *context.AmfRan, message *ngapType.NGAPPD
40404066
}
40414067
}
40424068

4069+
if rANUENGAPID == nil {
4070+
ran.Log.Errorln("RANUENGAPID IE missing from UplinkRanStatusTransfer")
4071+
return
4072+
}
4073+
40434074
ranUe = ran.RanUeFindByRanUeNgapID(rANUENGAPID.Value)
40444075
if ranUe == nil {
40454076
ran.Log.Errorf("No UE Context[RanUeNgapID: %d]", rANUENGAPID.Value)

‎ngap/handler_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,72 @@ func TestHandleHandoverNotifyIgnoresMissingIDs(t *testing.T) {
7272
HandleHandoverNotify(ctxt.Background(), ran, pdu)
7373
}
7474

75+
func TestHandlePDUSessionResourceNotifyIgnoresMissingIEs(t *testing.T) {
76+
ran := &context.AmfRan{Log: zap.NewNop().Sugar()}
77+
pdu := &ngapType.NGAPPDU{
78+
Present: ngapType.NGAPPDUPresentInitiatingMessage,
79+
InitiatingMessage: &ngapType.InitiatingMessage{
80+
ProcedureCode: ngapType.ProcedureCode{Value: ngapType.ProcedureCodePDUSessionResourceNotify},
81+
Value: ngapType.InitiatingMessageValue{
82+
Present: ngapType.InitiatingMessagePresentPDUSessionResourceNotify,
83+
PDUSessionResourceNotify: &ngapType.PDUSessionResourceNotify{},
84+
},
85+
},
86+
}
87+
88+
defer func() {
89+
if recovered := recover(); recovered != nil {
90+
t.Fatalf("HandlePDUSessionResourceNotify panicked with missing IEs: %v", recovered)
91+
}
92+
}()
93+
94+
HandlePDUSessionResourceNotify(ctxt.Background(), ran, pdu)
95+
}
96+
97+
func TestHandleHandoverFailureIgnoresMissingIEs(t *testing.T) {
98+
ran := &context.AmfRan{Log: zap.NewNop().Sugar()}
99+
pdu := &ngapType.NGAPPDU{
100+
Present: ngapType.NGAPPDUPresentUnsuccessfulOutcome,
101+
UnsuccessfulOutcome: &ngapType.UnsuccessfulOutcome{
102+
ProcedureCode: ngapType.ProcedureCode{Value: ngapType.ProcedureCodeHandoverResourceAllocation},
103+
Value: ngapType.UnsuccessfulOutcomeValue{
104+
Present: ngapType.UnsuccessfulOutcomePresentHandoverResourceAllocation,
105+
HandoverResourceAllocation: &ngapType.HandoverFailure{},
106+
},
107+
},
108+
}
109+
110+
defer func() {
111+
if recovered := recover(); recovered != nil {
112+
t.Fatalf("HandleHandoverFailure panicked with missing IEs: %v", recovered)
113+
}
114+
}()
115+
116+
HandleHandoverFailure(ctxt.Background(), ran, pdu)
117+
}
118+
119+
func TestHandleUplinkRanStatusTransferIgnoresMissingIEs(t *testing.T) {
120+
ran := &context.AmfRan{Log: zap.NewNop().Sugar()}
121+
pdu := &ngapType.NGAPPDU{
122+
Present: ngapType.NGAPPDUPresentInitiatingMessage,
123+
InitiatingMessage: &ngapType.InitiatingMessage{
124+
ProcedureCode: ngapType.ProcedureCode{Value: ngapType.ProcedureCodeUplinkRANStatusTransfer},
125+
Value: ngapType.InitiatingMessageValue{
126+
Present: ngapType.InitiatingMessagePresentUplinkRANStatusTransfer,
127+
UplinkRANStatusTransfer: &ngapType.UplinkRANStatusTransfer{},
128+
},
129+
},
130+
}
131+
132+
defer func() {
133+
if recovered := recover(); recovered != nil {
134+
t.Fatalf("HandleUplinkRanStatusTransfer panicked with missing IEs: %v", recovered)
135+
}
136+
}()
137+
138+
HandleUplinkRanStatusTransfer(ran, pdu)
139+
}
140+
75141
func TestHandleHandoverRequestAcknowledgeIgnoresMissingAmfUeNgapID(t *testing.T) {
76142
ran := &context.AmfRan{Log: zap.NewNop().Sugar()}
77143
pdu := &ngapType.NGAPPDU{

0 commit comments

Comments
 (0)