🐛 Fix nil pointer dereferences in Azure provider#6945
Closed
Conversation
Add nil checks for Azure SDK pointer-typed Properties fields across multiple resources to prevent panics when API responses contain nil values. Files fixed: - advisor.go: guard r.Properties before accessing recommendations - aks.go: guard entry.Properties.PowerState, use convert.ToValue for entry.ID - cloud_defender.go: guard setting.Properties and containersPricing.Properties - compute.go: guard networkInterface.Properties, config.Properties, ip.ID - iam.go: guard roleDef.Properties and roleDef.ID - monitor.go: guard entry.Properties.Actions and .Condition - network.go: guard flowLog.Properties, vng.Properties.BgpSettings, vng.Properties.SKU, ipc.Properties, fw.Properties, ipConfig.Properties - sql.go: guard entry.Properties, entry.SKU, vaSettings.Properties, vaSettings.Properties.RecurringScans Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![mondoo-code-review[bot]](https://avatars.githubusercontent.com/in/2834902?s=60&v=4){kind=small}
There was a problem hiding this comment.
Azure provider will still crash on nil Properties in firewall resources despite partial fix.
Additional findings (file/line not in diff):
- 🔴
providers/azure/resources/network.go:2279— Nil pointer dereference not fixed here.fw.Properties.SKU.Tier,fw.Properties.SKU.Name,fw.Properties.ProvisioningState, andfw.Properties.ThreatIntelModeare accessed without checkingfw.Propertiesorfw.Properties.SKUfor nil — the same class of bug this PR is fixing elsewhere.
Apply the same pattern used for virtualNetworkGateways (line ~1177):
args := map[string]*llx.RawData{
// ... safe fields only ...
}
if fw.Properties != nil {
args["provisioningState"] = llx.StringDataPtr((*string)(fw.Properties.ProvisioningState))
args["threatIntelMode"] = llx.StringDataPtr((*string)(fw.Properties.ThreatIntelMode))
if fw.Properties.SKU != nil {
args["skuTier"] = llx.StringDataPtr((*string)(fw.Properties.SKU.Tier))
args["skuName"] = llx.StringDataPtr((*string)(fw.Properties.SKU.Name))
}
}- 🔴
providers/azure/resources/network.go:2288— Two remaining nil dereferences:
- Line 2288:
fw.Properties.ManagementIPConfigurationis accessed without first checkingfw.Properties != nil. - Line 2299:
ipConfig.Properties.PrivateIPAddressis accessed without checkingipConfig.Properties != nil— the exact bug fixed for the IP configs loop above (line 2200-2202).
Fix:
if fw.Properties != nil && fw.Properties.ManagementIPConfiguration != nil {
ipConfig := fw.Properties.ManagementIPConfiguration
// ...
var privateIP *llx.RawData = llx.NilData
if ipConfig.Properties != nil {
privateIP = llx.StringDataPtr(ipConfig.Properties.PrivateIPAddress)
}
// use privateIP in CreateResource
}
Contributor
Address review: guard fw.Properties.SKU, fw.Properties.ProvisioningState, fw.Properties.ThreatIntelMode, fw.Properties.ManagementIPConfiguration, and ipConfig.Properties.PrivateIPAddress in the management IP config. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
+2194
to
+2218
| if fw.Properties != nil { | ||
| for _, ipConfig := range fw.Properties.IPConfigurations { | ||
| ipProps, err := convert.JsonToDict(ipConfig.Properties) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| var privateIP *llx.RawData = llx.NilData | ||
| if ipConfig.Properties != nil { | ||
| privateIP = llx.StringDataPtr(ipConfig.Properties.PrivateIPAddress) | ||
| } | ||
| mqlIpConfig, err := CreateResource(runtime, "azure.subscription.networkService.firewall.ipConfig", | ||
| map[string]*llx.RawData{ | ||
| "id": llx.StringDataPtr(ipConfig.ID), | ||
| "name": llx.StringDataPtr(ipConfig.Name), | ||
| "etag": llx.StringDataPtr(ipConfig.Etag), | ||
| "privateIpAddress": privateIP, | ||
| "properties": llx.DictData(ipProps), | ||
| }) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| ipConfigs = append(ipConfigs, mqlIpConfig) | ||
| } | ||
| natRules = append(natRules, mqlNatRule) | ||
| } | ||
| for _, networkRule := range fw.Properties.NetworkRuleCollections { | ||
| props, err := convert.JsonToDict(networkRule.Properties) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| mqlNetworkRule, err := CreateResource(runtime, "azure.subscription.networkService.firewall.networkRule", | ||
| map[string]*llx.RawData{ | ||
| "id": llx.StringDataPtr(networkRule.ID), | ||
| "name": llx.StringDataPtr(networkRule.Name), | ||
| "etag": llx.StringDataPtr(networkRule.Etag), | ||
| "properties": llx.DictData(props), | ||
| }) | ||
| if err != nil { | ||
| return nil, err | ||
| if fw.Properties != nil { |
There was a problem hiding this comment.
🔵 suggestion — The two separate if fw.Properties != nil blocks (one for IPConfigurations and another for NatRuleCollections/NetworkRuleCollections/ApplicationRuleCollections) could be consolidated into a single block to reduce duplication.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Propertiesfields across 8 resource files to prevent panics when API responses contain nil valuesentry.Properties.PowerState.Code,vng.Properties.BgpSettings.BgpPeeringAddresses,flowLog.Properties.RetentionPolicy, etc.Test plan
go build ./...passes inproviders/azure/(verified locally)go vet ./...passes inproviders/azure/(verified locally)mql shell azureand query affected resources:azure.subscription.advisor.recommendations,azure.subscription.aksService.clusters,azure.subscription.cloudDefender.monitoringAgentAutoProvision,azure.subscription.networkService.virtualNetworkGateways,azure.subscription.networkService.watchers { flowLogs },azure.subscription.sqlService.databases🤖 Generated with Claude Code