Skip to content

Commit c892169

Browse files
dustin-deckerdustin-decker
authored
fix(postman): prevent infinite recursion in variable substitution (#4145)
* fix(postman): prevent infinite recursion in variable substitution Add recursion depth limit and self-reference detection to the buildSubstitution function to prevent hanging when processing complex variable patterns. Includes comprehensive tests for edge cases. * pr review improvements
1 parent a230f95 commit c892169

File tree

3 files changed

+158
-25
lines changed

3 files changed

+158
-25
lines changed

‎pkg/sources/postman/postman.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ func (s *Source) scanEvent(ctx context.Context, chunksChan chan *sources.Chunk,
434434
metadata.LocationType = source_metadatapb.PostmanLocationType_COLLECTION_SCRIPT
435435
}
436436

437-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(metadata, data)), metadata)
437+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(metadata, data, DefaultMaxRecursionDepth)), metadata)
438438
metadata.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
439439
}
440440

@@ -532,7 +532,7 @@ func (s *Source) scanAuth(ctx context.Context, chunksChan chan *sources.Chunk, m
532532
} else if strings.Contains(m.Type, COLLECTION_TYPE) {
533533
m.LocationType = source_metadatapb.PostmanLocationType_COLLECTION_AUTHORIZATION
534534
}
535-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, authData)), m)
535+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, authData, DefaultMaxRecursionDepth)), m)
536536
m.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
537537
}
538538

@@ -555,7 +555,7 @@ func (s *Source) scanHTTPRequest(ctx context.Context, chunksChan chan *sources.C
555555
metadata.Type = originalType + " > header"
556556
metadata.Link = metadata.Link + "?tab=headers"
557557
metadata.LocationType = source_metadatapb.PostmanLocationType_REQUEST_HEADER
558-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(metadata, strings.Join(r.HeaderString, " "))), metadata)
558+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(metadata, strings.Join(r.HeaderString, " "), DefaultMaxRecursionDepth)), metadata)
559559
metadata.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
560560
}
561561

@@ -564,7 +564,7 @@ func (s *Source) scanHTTPRequest(ctx context.Context, chunksChan chan *sources.C
564564
// Note: query parameters are handled separately
565565
u := fmt.Sprintf("%s://%s/%s", r.URL.Protocol, strings.Join(r.URL.Host, "."), strings.Join(r.URL.Path, "/"))
566566
metadata.LocationType = source_metadatapb.PostmanLocationType_REQUEST_URL
567-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(metadata, u)), metadata)
567+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(metadata, u, DefaultMaxRecursionDepth)), metadata)
568568
metadata.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
569569
}
570570

@@ -615,13 +615,13 @@ func (s *Source) scanRequestBody(ctx context.Context, chunksChan chan *sources.C
615615
m.Type = originalType + " > raw"
616616
data := b.Raw
617617
m.LocationType = source_metadatapb.PostmanLocationType_REQUEST_BODY_RAW
618-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, data)), m)
618+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, data, DefaultMaxRecursionDepth)), m)
619619
m.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
620620
case "graphql":
621621
m.Type = originalType + " > graphql"
622622
data := b.GraphQL.Query + " " + b.GraphQL.Variables
623623
m.LocationType = source_metadatapb.PostmanLocationType_REQUEST_BODY_GRAPHQL
624-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, data)), m)
624+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, data, DefaultMaxRecursionDepth)), m)
625625
m.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
626626
}
627627
}
@@ -647,15 +647,15 @@ func (s *Source) scanHTTPResponse(ctx context.Context, chunksChan chan *sources.
647647
m.Type = originalType + " > response header"
648648
// TODO Note: for now, links to Postman responses do not include a more granular tab for the params/header/body, but when they do, we will need to update the metadata.Link info
649649
m.LocationType = source_metadatapb.PostmanLocationType_RESPONSE_HEADER
650-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, strings.Join(response.HeaderString, " "))), m)
650+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, strings.Join(response.HeaderString, " "), DefaultMaxRecursionDepth)), m)
651651
m.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
652652
}
653653

654654
// Body in a response is just a string
655655
if response.Body != "" {
656656
m.Type = originalType + " > response body"
657657
m.LocationType = source_metadatapb.PostmanLocationType_RESPONSE_BODY
658-
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, response.Body)), m)
658+
s.scanData(ctx, chunksChan, s.formatAndInjectKeywords(s.buildSubstituteSet(m, response.Body, DefaultMaxRecursionDepth)), m)
659659
m.LocationType = source_metadatapb.PostmanLocationType_UNKNOWN_POSTMAN
660660
}
661661

@@ -688,7 +688,7 @@ func (s *Source) scanVariableData(ctx context.Context, chunksChan chan *sources.
688688
if valStr == "" {
689689
continue
690690
}
691-
values = append(values, s.buildSubstituteSet(m, valStr)...)
691+
values = append(values, s.buildSubstituteSet(m, valStr, DefaultMaxRecursionDepth)...)
692692
}
693693

694694
m.FieldType = m.Type + " variables"

‎pkg/sources/postman/substitution.go

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import (
88

99
var subRe = regexp.MustCompile(`\{\{[^{}]+\}\}`)
1010

11+
// DefaultMaxRecursionDepth is the default maximum recursion depth for variable substitution
12+
const DefaultMaxRecursionDepth = 2
13+
1114
type VariableInfo struct {
1215
value string
1316
Metadata Metadata
@@ -47,11 +50,14 @@ func (s *Source) formatAndInjectKeywords(data []string) string {
4750
return strings.Join(ret, "")
4851
}
4952

50-
func (s *Source) buildSubstituteSet(metadata Metadata, data string) []string {
53+
// buildSubstituteSet creates a set of substitutions for the given data
54+
// maxRecursionDepth is the maximum recursion depth to use for variable substitution
55+
func (s *Source) buildSubstituteSet(metadata Metadata, data string, maxRecursionDepth int) []string {
5156
var ret []string
5257
combos := make(map[string]struct{})
5358

54-
s.buildSubstitution(data, metadata, &combos)
59+
// Call buildSubstitution with initial depth of 0 and the maxRecursionDepth
60+
s.buildSubstitution(data, metadata, &combos, 0, maxRecursionDepth)
5561

5662
for combo := range combos {
5763
ret = append(ret, combo)
@@ -63,26 +69,55 @@ func (s *Source) buildSubstituteSet(metadata Metadata, data string) []string {
6369
return ret
6470
}
6571

66-
func (s *Source) buildSubstitution(data string, metadata Metadata, combos *map[string]struct{}) {
72+
// buildSubstitution performs variable substitution with a maximum recursion depth
73+
// depth is the current recursion depth
74+
// maxRecursionDepth is the maximum recursion depth to use for variable substitution
75+
func (s *Source) buildSubstitution(data string, metadata Metadata, combos *map[string]struct{}, depth int, maxRecursionDepth int) {
76+
// Limit recursion depth to prevent stack overflow
77+
if depth > maxRecursionDepth {
78+
(*combos)[data] = struct{}{}
79+
return
80+
}
81+
6782
matches := removeDuplicateStr(subRe.FindAllString(data, -1))
83+
if len(matches) == 0 {
84+
// No more substitutions to make, add to combos
85+
(*combos)[data] = struct{}{}
86+
return
87+
}
88+
89+
substitutionMade := false
6890
for _, match := range matches {
69-
for _, slice := range s.sub.variables[strings.Trim(match, "{}")] {
70-
if slice.Metadata.CollectionInfo.PostmanID != "" && slice.Metadata.CollectionInfo.PostmanID != metadata.CollectionInfo.PostmanID {
91+
varName := strings.Trim(match, "{}")
92+
slices := s.sub.variables[varName]
93+
if len(slices) == 0 {
94+
continue
95+
}
96+
97+
for _, slice := range slices {
98+
if slice.Metadata.CollectionInfo.PostmanID != "" &&
99+
slice.Metadata.CollectionInfo.PostmanID != metadata.CollectionInfo.PostmanID {
71100
continue
72101
}
73102

74-
// to ensure we don't infinitely recurse, we will trim all `{{}}` from the values before replacement.
75-
// this is to prevent the case where a variable is replaced with a value that contains the same variable causing
76-
// an infinite loop
77-
removedBrackets := strings.ReplaceAll(strings.ReplaceAll(slice.value, "{{", ""), "}}", "")
103+
// Prevent self-referential variables
104+
if strings.Contains(slice.value, match) {
105+
continue
106+
}
78107

79-
d := strings.ReplaceAll(data, match, removedBrackets)
80-
s.buildSubstitution(d, metadata, combos)
108+
// Use the actual value for substitution, not just the stripped version
109+
d := strings.ReplaceAll(data, match, slice.value)
110+
111+
// Only mark substitution as made if we actually changed something
112+
if d != data {
113+
substitutionMade = true
114+
s.buildSubstitution(d, metadata, combos, depth+1, maxRecursionDepth)
115+
}
81116
}
82117
}
83118

84-
if len(matches) == 0 {
85-
// add to combos
119+
// If no substitutions were made, add the current data
120+
if !substitutionMade {
86121
(*combos)[data] = struct{}{}
87122
}
88123
}

‎pkg/sources/postman/substitution_test.go

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ func TestSource_BuildSubstituteSet(t *testing.T) {
8484
{"{{var2}}", []string{"value2"}},
8585
{"{{var1}}:{{var2}}", []string{"value1:value2"}},
8686
{"no variables", []string{"no variables"}},
87-
{"{{var1}}:{{continuation_token}}", []string{"value1:'continuation_token'"}},
88-
{"{{var1}}:{{continuation_token2}}", []string{"value1:'{continuation_token2}'"}},
87+
{"{{var1}}:{{continuation_token}}", []string{"value1:{{continuation_token}}"}},
88+
{"{{var1}}:{{continuation_token2}}", []string{"value1:{{continuation_token2}}"}},
8989
}
9090

9191
for _, tc := range testCases {
92-
result := s.buildSubstituteSet(metadata, tc.data)
92+
result := s.buildSubstituteSet(metadata, tc.data, DefaultMaxRecursionDepth)
9393
if !reflect.DeepEqual(result, tc.expected) {
9494
t.Errorf("Expected substitution set: %v, got: %v", tc.expected, result)
9595
}
@@ -156,3 +156,101 @@ func TestSource_FormatAndInjectKeywords(t *testing.T) {
156156
}
157157
}
158158
}
159+
160+
func TestSource_BuildSubstitution_RecursionLimit(t *testing.T) {
161+
s := &Source{
162+
sub: NewSubstitution(),
163+
}
164+
metadata := Metadata{
165+
Type: ENVIRONMENT_TYPE,
166+
}
167+
168+
// Setup test cases
169+
170+
// 1. Self-referential variable (should be skipped)
171+
s.sub.add(metadata, "self_ref", "{{self_ref}}")
172+
173+
// 2. Nested variables for testing recursion depth
174+
s.sub.add(metadata, "var1", "{{var2}}")
175+
s.sub.add(metadata, "var2", "{{var3}}")
176+
s.sub.add(metadata, "var3", "{{var4}}")
177+
s.sub.add(metadata, "var4", "{{var5}}")
178+
s.sub.add(metadata, "var5", "{{var6}}")
179+
s.sub.add(metadata, "var6", "{{var7}}")
180+
s.sub.add(metadata, "var7", "{{var8}}")
181+
s.sub.add(metadata, "var8", "{{var9}}")
182+
s.sub.add(metadata, "var9", "{{var10}}")
183+
s.sub.add(metadata, "var10", "{{var11}}")
184+
s.sub.add(metadata, "var11", "{{var12}}")
185+
s.sub.add(metadata, "var12", "final_value")
186+
187+
// 3. Circular reference
188+
s.sub.add(metadata, "circular1", "{{circular2}}")
189+
s.sub.add(metadata, "circular2", "{{circular3}}")
190+
s.sub.add(metadata, "circular3", "{{circular1}}")
191+
192+
testCases := []struct {
193+
name string
194+
data string
195+
expected []string
196+
maxDepth int
197+
}{
198+
{
199+
name: "Self-referential variable",
200+
data: "{{self_ref}}",
201+
expected: []string{"{{self_ref}}"},
202+
},
203+
{
204+
name: "Nested variables within depth limit",
205+
data: "{{var8}}",
206+
expected: []string{"{{var11}}"},
207+
},
208+
{
209+
name: "Nested variables exceeding depth limit",
210+
data: "{{var1}}",
211+
expected: []string{"{{var4}}"},
212+
},
213+
{
214+
name: "Circular reference",
215+
data: "{{circular1}}",
216+
expected: []string{"{{circular1}}"},
217+
},
218+
{
219+
name: "Custom recursion depth limit (5)",
220+
data: "{{var1}}",
221+
expected: []string{"{{var7}}"},
222+
maxDepth: 5,
223+
},
224+
}
225+
226+
for _, tc := range testCases {
227+
t.Run(tc.name, func(t *testing.T) {
228+
combos := make(map[string]struct{})
229+
230+
// Use custom maxDepth if provided, otherwise use default
231+
if tc.maxDepth > 0 {
232+
s.buildSubstitution(tc.data, metadata, &combos, 0, tc.maxDepth)
233+
} else {
234+
s.buildSubstitution(tc.data, metadata, &combos, 0, DefaultMaxRecursionDepth)
235+
}
236+
237+
var result []string
238+
for combo := range combos {
239+
result = append(result, combo)
240+
}
241+
242+
// If no substitutions were made, the original data should be returned
243+
if len(result) == 0 {
244+
result = []string{tc.data}
245+
}
246+
247+
// Sort both slices for consistent comparison
248+
sort.Strings(result)
249+
sort.Strings(tc.expected)
250+
251+
if !reflect.DeepEqual(result, tc.expected) {
252+
t.Errorf("Expected: %v, got: %v", tc.expected, result)
253+
}
254+
})
255+
}
256+
}

0 commit comments

Comments
 (0)