Skip to content

Commit 76ef215

Browse files
jdamickJeffrey Damick
authored andcommitted
Merge branch 'coredns:master' into master
2 parents 02a5d37 + 543b91b commit 76ef215

File tree

3 files changed

+123
-7
lines changed

3 files changed

+123
-7
lines changed

caddy.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (i *Instance) Stop() error {
149149
instancesMu.Lock()
150150
for j, other := range instances {
151151
if other == i {
152-
instances[j] = nil
152+
slices.Delete(instances, j, j+1)
153153
instances = append(instances[:j], instances[j+1:]...)
154154
break
155155
}
@@ -503,7 +503,7 @@ func startWithListenerFds(cdyfile Input, inst *Instance, restartFds map[string][
503503
instancesMu.Lock()
504504
for i, otherInst := range instances {
505505
if otherInst == inst {
506-
instances[i] = nil
506+
slices.Delete(instances, i, i+1)
507507
instances = append(instances[:i], instances[i+1:]...)
508508
break
509509
}

caddyfile/parse.go

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,22 @@ func allTokens(input io.Reader) ([]Token, error) {
5151

5252
type parser struct {
5353
Dispenser
54-
block ServerBlock // current server block being parsed
55-
validDirectives []string // a directive must be valid or it's an error
56-
eof bool // if we encounter a valid EOF in a hard place
57-
definedSnippets map[string][]Token
54+
block ServerBlock // current server block being parsed
55+
validDirectives []string // a directive must be valid or it's an error
56+
eof bool // if we encounter a valid EOF in a hard place
57+
definedSnippets map[string][]Token
58+
snippetExpansions int // counts snippet imports expanded during this parse
59+
fileExpansions int // counts file/glob imports expanded during this parse
5860
}
5961

62+
// maxSnippetExpansions is a hard cap to prevent excessively deep or cyclic snippet imports.
63+
// set as a variable to allow modifications for testing
64+
var maxSnippetExpansions = 10000
65+
66+
// maxFileExpansions is a hard cap to prevent excessively deep or cyclic file imports.
67+
// set as a variable to allow modifications for testing
68+
var maxFileExpansions = 100000
69+
6070
func (p *parser) parseAll() ([]ServerBlock, error) {
6171
var blocks []ServerBlock
6272

@@ -108,6 +118,16 @@ func (p *parser) begin() error {
108118
if err != nil {
109119
return err
110120
}
121+
// minimal guard: detect trivial self-import in snippet body
122+
for i := 0; i+1 < len(tokens); i++ {
123+
if tokens[i].Text == "import" {
124+
// Only consider it an import directive if at start of a line
125+
atLineStart := i == 0 || tokens[i-1].File != tokens[i].File || tokens[i-1].Line != tokens[i].Line
126+
if atLineStart && replaceEnvVars(tokens[i+1].Text) == name {
127+
return p.Errf("maximum snippet import depth (%d) exceeded", maxSnippetExpansions)
128+
}
129+
}
130+
}
111131
p.definedSnippets[name] = tokens
112132
// empty block keys so we don't save this block as a real server.
113133
p.block.Keys = nil
@@ -245,10 +265,18 @@ func (p *parser) doImport() error {
245265
tokensAfter := p.tokens[p.cursor+1:]
246266
var importedTokens []Token
247267

248-
// first check snippets. That is a simple, non-recursive replacement
268+
// first check snippets. Count expansion and enforce cap.
249269
if p.definedSnippets != nil && p.definedSnippets[importPattern] != nil {
270+
if p.snippetExpansions >= maxSnippetExpansions {
271+
return p.Errf("maximum snippet import depth (%d) exceeded", maxSnippetExpansions)
272+
}
273+
p.snippetExpansions++
250274
importedTokens = p.definedSnippets[importPattern]
251275
} else {
276+
if p.fileExpansions >= maxFileExpansions {
277+
return p.Errf("maximum file import depth (%d) exceeded", maxSnippetExpansions)
278+
}
279+
p.fileExpansions++
252280
// make path relative to the file of the _token_ being processed rather
253281
// than current working directory (issue #867) and then use glob to get
254282
// list of matching filenames

caddyfile/parse_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,20 @@
1515
package caddyfile
1616

1717
import (
18+
"fmt"
1819
"io/ioutil"
1920
"os"
2021
"path/filepath"
2122
"strings"
2223
"testing"
2324
)
2425

26+
func init() {
27+
// set a lower limit for testing only
28+
maxSnippetExpansions = 10
29+
maxFileExpansions = 10
30+
}
31+
2532
func TestAllTokens(t *testing.T) {
2633
tests := []struct {
2734
name string
@@ -744,3 +751,84 @@ func TestSnippetAcrossMultipleFiles(t *testing.T) {
744751
t.Errorf("Expected argument to be '%s' but was '%s'", expected, actual)
745752
}
746753
}
754+
755+
func TestSnippetImportCycle(t *testing.T) {
756+
tests := []struct {
757+
name string
758+
input string
759+
}{
760+
{
761+
name: "direct self-import",
762+
input: `
763+
(loop) {
764+
import loop
765+
}
766+
import loop
767+
`,
768+
},
769+
{
770+
name: "self-import via recursion",
771+
input: `
772+
(loop) {
773+
{ }
774+
import loop
775+
}
776+
import loop
777+
`,
778+
},
779+
}
780+
781+
for _, tt := range tests {
782+
t.Run(tt.name, func(t *testing.T) {
783+
p := testParser(tt.input)
784+
_, err := p.parseAll()
785+
if err == nil {
786+
t.Fatalf("expected error for snippet self-import cycle, got nil")
787+
}
788+
})
789+
}
790+
}
791+
792+
func TestFileImportCycleError(t *testing.T) {
793+
fileA := writeStringToTempFileOrDie(t, "")
794+
defer os.Remove(fileA)
795+
fileB := writeStringToTempFileOrDie(t, "")
796+
defer os.Remove(fileB)
797+
798+
if err := ioutil.WriteFile(fileA, []byte("import "+fileB), 0644); err != nil {
799+
t.Fatal(err)
800+
}
801+
if err := ioutil.WriteFile(fileB, []byte("import "+fileA), 0644); err != nil {
802+
t.Fatal(err)
803+
}
804+
805+
p := testParser("import " + fileA)
806+
_, err := p.parseAll()
807+
if err == nil {
808+
t.Fatalf("expected error for file import cycle, got nil")
809+
}
810+
}
811+
812+
func TestFileImportDir(t *testing.T) {
813+
dir, err := ioutil.TempDir("", t.Name())
814+
if err != nil {
815+
t.Fatal(err)
816+
}
817+
defer os.RemoveAll(dir)
818+
819+
// create 10x the maxFileExpansions files
820+
// a single import with a glob should not error
821+
for i := 0; i < maxFileExpansions*10; i++ {
822+
fp := filepath.Join(dir, filepath.Base(dir)+"_"+fmt.Sprintf("%d", i))
823+
if err := ioutil.WriteFile(fp, []byte(""), 0644); err != nil {
824+
t.Fatal(err)
825+
}
826+
}
827+
828+
input := "import " + filepath.Join(dir, "*")
829+
p := testParser(input)
830+
_, err = p.parseAll()
831+
if err != nil {
832+
t.Fatalf("unexpected error importing temp dir via glob: %v", err)
833+
}
834+
}

0 commit comments

Comments
 (0)