Skip to content

Commit 77bab7b

Browse files
nikpivkinnikpivkin
authored
fix(misconf): correctly parse empty port ranges in google_compute_firewall (#9237)
Signed-off-by: nikpivkin <[email protected]>
1 parent 2c05882 commit 77bab7b

File tree

11 files changed

+295
-175
lines changed

11 files changed

+295
-175
lines changed

‎pkg/iac/adapters/arm/network/adapt.go‎

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
package network
22

33
import (
4-
"strconv"
5-
"strings"
6-
4+
"github.com/aquasecurity/trivy/pkg/iac/adapters/common"
75
"github.com/aquasecurity/trivy/pkg/iac/providers/azure/network"
86
"github.com/aquasecurity/trivy/pkg/iac/scanners/azure"
97
iacTypes "github.com/aquasecurity/trivy/pkg/iac/types"
108
)
119

10+
func parsePortRange(input string, meta iacTypes.Metadata) common.PortRange {
11+
return common.ParsePortRange(input, meta, common.WithWildcard())
12+
}
13+
1214
func Adapt(deployment azure.Deployment) network.Network {
1315
return network.Network{
1416
SecurityGroups: adaptSecurityGroups(deployment),
@@ -42,20 +44,30 @@ func adaptSecurityGroupRule(resource azure.Resource) network.SecurityGroupRule {
4244
sourceAddressPrefixes := resource.Properties.GetMapValue("sourceAddressPrefixes").AsStringValuesList("")
4345
sourceAddressPrefixes = append(sourceAddressPrefixes, resource.Properties.GetMapValue("sourceAddressPrefix").AsStringValue("", resource.Metadata))
4446

45-
var sourcePortRanges []network.PortRange
47+
var sourcePortRanges []common.PortRange
4648
for _, portRange := range resource.Properties.GetMapValue("sourcePortRanges").AsList() {
47-
sourcePortRanges = append(sourcePortRanges, expandRange(portRange.AsString(), resource.Metadata))
49+
rng := parsePortRange(portRange.AsString(), resource.Metadata)
50+
if rng.Valid() {
51+
sourcePortRanges = append(sourcePortRanges, rng)
52+
}
53+
}
54+
if rng := parsePortRange(resource.Properties.GetMapValue("sourcePortRange").AsString(), resource.Metadata); rng.Valid() {
55+
sourcePortRanges = append(sourcePortRanges, rng)
4856
}
49-
sourcePortRanges = append(sourcePortRanges, expandRange(resource.Properties.GetMapValue("sourcePortRange").AsString(), resource.Metadata))
5057

5158
destinationAddressPrefixes := resource.Properties.GetMapValue("destinationAddressPrefixes").AsStringValuesList("")
5259
destinationAddressPrefixes = append(destinationAddressPrefixes, resource.Properties.GetMapValue("destinationAddressPrefix").AsStringValue("", resource.Metadata))
5360

54-
var destinationPortRanges []network.PortRange
61+
var destinationPortRanges []common.PortRange
5562
for _, portRange := range resource.Properties.GetMapValue("destinationPortRanges").AsList() {
56-
destinationPortRanges = append(destinationPortRanges, expandRange(portRange.AsString(), resource.Metadata))
63+
rng := parsePortRange(portRange.AsString(), resource.Metadata)
64+
if rng.Valid() {
65+
destinationPortRanges = append(destinationPortRanges, rng)
66+
}
67+
}
68+
if rng := parsePortRange(resource.Properties.GetMapValue("destinationPortRange").AsString(), resource.Metadata); rng.Valid() {
69+
destinationPortRanges = append(destinationPortRanges, rng)
5770
}
58-
destinationPortRanges = append(destinationPortRanges, expandRange(resource.Properties.GetMapValue("destinationPortRange").AsString(), resource.Metadata))
5971

6072
allow := iacTypes.BoolDefault(false, resource.Metadata)
6173
if resource.Properties.GetMapValue("access").AsString() == "Allow" {
@@ -96,31 +108,3 @@ func adaptNetworkWatcherFlowLog(resource azure.Resource) network.NetworkWatcherF
96108
},
97109
}
98110
}
99-
100-
func expandRange(r string, m iacTypes.Metadata) network.PortRange {
101-
start := 0
102-
end := 65535
103-
switch {
104-
case r == "*":
105-
case strings.Contains(r, "-"):
106-
if parts := strings.Split(r, "-"); len(parts) == 2 {
107-
if p1, err := strconv.ParseInt(parts[0], 10, 32); err == nil {
108-
start = int(p1)
109-
}
110-
if p2, err := strconv.ParseInt(parts[1], 10, 32); err == nil {
111-
end = int(p2)
112-
}
113-
}
114-
default:
115-
if val, err := strconv.ParseInt(r, 10, 32); err == nil {
116-
start = int(val)
117-
end = int(val)
118-
}
119-
}
120-
121-
return network.PortRange{
122-
Metadata: m,
123-
Start: iacTypes.Int(start, m),
124-
End: iacTypes.Int(end, m),
125-
}
126-
}

‎pkg/iac/adapters/arm/network/adapt_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
"github.com/aquasecurity/trivy/pkg/iac/adapters/arm/adaptertest"
7+
"github.com/aquasecurity/trivy/pkg/iac/adapters/common"
78
"github.com/aquasecurity/trivy/pkg/iac/providers/azure/network"
89
"github.com/aquasecurity/trivy/pkg/iac/types"
910
)
@@ -42,9 +43,7 @@ func TestAdapt(t *testing.T) {
4243
SecurityGroups: []network.SecurityGroup{{
4344
Rules: []network.SecurityGroupRule{{
4445
DestinationAddresses: []types.StringValue{types.StringTest("")},
45-
DestinationPorts: []network.PortRange{{Start: types.IntTest(0), End: types.IntTest(65535)}},
4646
SourceAddresses: []types.StringValue{types.StringTest("")},
47-
SourcePorts: []network.PortRange{{Start: types.IntTest(0), End: types.IntTest(65535)}},
4847
}},
4948
}},
5049
},
@@ -111,7 +110,7 @@ func TestAdapt(t *testing.T) {
111110
types.StringTest("10.0.2.0/24"),
112111
types.StringTest("10.0.0.0/24"),
113112
},
114-
SourcePorts: []network.PortRange{
113+
SourcePorts: []common.PortRange{
115114
{
116115
Start: types.IntTest(1000),
117116
End: types.IntTest(2000),
@@ -130,7 +129,7 @@ func TestAdapt(t *testing.T) {
130129
types.StringTest("172.16.2.0/24"),
131130
types.StringTest("172.16.0.0/16"),
132131
},
133-
DestinationPorts: []network.PortRange{
132+
DestinationPorts: []common.PortRange{
134133
{
135134
Start: types.IntTest(8080),
136135
End: types.IntTest(8080),

‎pkg/iac/adapters/common/network.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package common
2+
3+
import (
4+
"strconv"
5+
"strings"
6+
7+
iacTypes "github.com/aquasecurity/trivy/pkg/iac/types"
8+
)
9+
10+
type PortRange struct {
11+
Metadata iacTypes.Metadata
12+
Start iacTypes.IntValue
13+
End iacTypes.IntValue
14+
}
15+
16+
func NewPortRange(start, end int, meta iacTypes.Metadata) PortRange {
17+
return PortRange{
18+
Metadata: meta,
19+
Start: iacTypes.Int(start, meta),
20+
End: iacTypes.Int(end, meta),
21+
}
22+
}
23+
24+
func FullPortRange(meta iacTypes.Metadata) PortRange {
25+
return NewPortRange(0, 65535, meta)
26+
}
27+
28+
func InvalidPortRange(meta iacTypes.Metadata) PortRange {
29+
return NewPortRange(-1, -1, meta)
30+
}
31+
32+
func (r PortRange) Valid() bool {
33+
return !r.Start.EqualTo(-1) && !r.End.EqualTo(-1)
34+
}
35+
36+
type parseConfig struct {
37+
allowWildcard bool
38+
}
39+
40+
type ParseOption func(*parseConfig)
41+
42+
func WithWildcard() ParseOption {
43+
return func(cfg *parseConfig) {
44+
cfg.allowWildcard = true
45+
}
46+
}
47+
48+
func ParsePortRange(input string, meta iacTypes.Metadata, opts ...ParseOption) PortRange {
49+
cfg := &parseConfig{}
50+
for _, opt := range opts {
51+
opt(cfg)
52+
}
53+
54+
input = strings.TrimSpace(input)
55+
56+
switch {
57+
case input == "*" && cfg.allowWildcard:
58+
return FullPortRange(meta)
59+
case strings.Contains(input, "-"):
60+
parts := strings.SplitN(input, "-", 2)
61+
if len(parts) != 2 {
62+
return InvalidPortRange(meta)
63+
}
64+
start, err1 := strconv.Atoi(strings.TrimSpace(parts[0]))
65+
end, err2 := strconv.Atoi(strings.TrimSpace(parts[1]))
66+
if err1 != nil || err2 != nil {
67+
return InvalidPortRange(meta)
68+
}
69+
return NewPortRange(start, end, meta)
70+
71+
default:
72+
val, err := strconv.Atoi(input)
73+
if err != nil {
74+
return InvalidPortRange(meta)
75+
}
76+
return NewPortRange(val, val, meta)
77+
}
78+
}

‎pkg/iac/adapters/common/network_test.go‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
package common_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
"github.com/aquasecurity/trivy/pkg/iac/adapters/common"
9+
iacTypes "github.com/aquasecurity/trivy/pkg/iac/types"
10+
)
11+
12+
func TestParsePortRange(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
input string
16+
options []common.ParseOption
17+
expected common.PortRange
18+
valid bool
19+
}{
20+
{
21+
name: "single port",
22+
input: "80",
23+
expected: common.PortRange{
24+
Start: iacTypes.IntTest(80),
25+
End: iacTypes.IntTest(80),
26+
},
27+
valid: true,
28+
},
29+
{
30+
name: "port range",
31+
input: "1000-2000",
32+
expected: common.PortRange{
33+
Start: iacTypes.IntTest(1000),
34+
End: iacTypes.IntTest(2000),
35+
},
36+
valid: true,
37+
},
38+
{
39+
name: "port range with spaces",
40+
input: " 22 - 80 ",
41+
expected: common.PortRange{
42+
Start: iacTypes.IntTest(22),
43+
End: iacTypes.IntTest(80),
44+
},
45+
valid: true,
46+
},
47+
{
48+
name: "wildcard allowed",
49+
input: "*",
50+
options: []common.ParseOption{common.WithWildcard()},
51+
expected: common.PortRange{
52+
Start: iacTypes.IntTest(0),
53+
End: iacTypes.IntTest(65535),
54+
},
55+
valid: true,
56+
},
57+
{
58+
name: "wildcard disallowed",
59+
input: "*",
60+
valid: false,
61+
},
62+
{
63+
name: "invalid string",
64+
input: "abc",
65+
valid: false,
66+
},
67+
{
68+
name: "incomplete range",
69+
input: "80-",
70+
valid: false,
71+
},
72+
{
73+
name: "double dash",
74+
input: "--",
75+
valid: false,
76+
},
77+
}
78+
79+
for _, tc := range tests {
80+
t.Run(tc.name, func(t *testing.T) {
81+
meta := iacTypes.NewTestMetadata()
82+
pr := common.ParsePortRange(tc.input, meta, tc.options...)
83+
84+
if tc.valid {
85+
assert.True(t, pr.Valid())
86+
tc.expected.Metadata = meta
87+
assert.Equal(t, tc.expected, pr)
88+
} else {
89+
assert.False(t, pr.Valid())
90+
}
91+
})
92+
}
93+
}

0 commit comments

Comments
 (0)