Repository: avenga/couper
Pull request #584 (2022-09-09 16:39:13)

check path patterns and base_paths
* check `endpoint` path pattern starting with `/`
* check `endpoint` path pattern and `server`/`api` `base_path` not to have `.` or `..` segment

---
<details>
    <summary>Reviewer checklist</summary>
    <ul>
        <li>Read PR description: a summary about the changes is required</li>
        <li>Changelog updated</li>
        <li>Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage</li>
        <li>Pulled branch, manually tested</li>
        <li>Verified requirements are met</li>
        <li>Reviewed the code</li>
        <li>Reviewed the related tests</li>
    </ul>
</details>



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d078e33a..d34a0fe7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
 * **Fixed**
   * [`form_body`, `headers` and `cookies`](./docs/REFERENCE.md#request) can now be properly [custom-logged](./docs/LOGS.md#custom-logging) ([#535](https://github.com/avenga/couper/pull/535))
   * Disallow empty path parameters ([#526](https://github.com/avenga/couper/pull/526))
+  * Disallow endpoint path patterns not starting with `/`, endpoint path patterns and `base_path`s having `.` or `..` segments ([#584](https://github.com/avenga/couper/pull/584))
   * Basic Auth client authentication with OAuth2 (client ID and secret must be URL encoded) ([#537](https://github.com/avenga/couper/pull/537))
   * Config validation, e.g. label-uniqueness checks ([#563](https://github.com/avenga/couper/pull/563))
   * [OIDC](https://docs.couper.io/configuration/block/oidc) not using referenced backends, if only specific backends (`configuration_backend`, `jwks_uri_backend`, `token_backend`, `userinfo_backend`) were configured ([#570](https://github.com/avenga/couper/pull/570))
diff --git a/config/configload/load.go b/config/configload/load.go
index f4a51483..fb179d48 100644
--- a/config/configload/load.go
+++ b/config/configload/load.go
@@ -173,7 +173,7 @@ func bodiesToConfig(parsedBodies []*hclsyntax.Body, srcBytes [][]byte, env strin
 		Blocks: configBlocks,
 	}
 
-	if err = validateBody(configBody, true); err != nil {
+	if err = validateBody(configBody, len(parsedBodies) > 1); err != nil {
 		return nil, err
 	}
 
diff --git a/config/configload/validate.go b/config/configload/validate.go
index f8bb92b8..51cbd017 100644
--- a/config/configload/validate.go
+++ b/config/configload/validate.go
@@ -104,7 +104,7 @@ func validateBody(body hcl.Body, afterMerge bool) error {
 			}
 		} else if outerBlock.Type == server {
 			uniqueEndpoints := make(map[string]struct{})
-			serverBasePath, err := getBasePath(outerBlock)
+			serverBasePath, err := getBasePath(outerBlock, afterMerge)
 			if err != nil {
 				return err
 			}
@@ -112,11 +112,11 @@ func validateBody(body hcl.Body, afterMerge bool) error {
 			serverBasePath = path.Join("/", serverBasePath)
 			for _, innerBlock := range outerBlock.Body.Blocks {
 				if innerBlock.Type == endpoint {
-					if err = registerEndpointPattern(uniqueEndpoints, serverBasePath, innerBlock); err != nil {
+					if err = registerEndpointPattern(uniqueEndpoints, serverBasePath, innerBlock, afterMerge); err != nil {
 						return err
 					}
 				} else if innerBlock.Type == api {
-					apiBasePath, err := getBasePath(innerBlock)
+					apiBasePath, err := getBasePath(innerBlock, afterMerge)
 					if err != nil {
 						return err
 					}
@@ -124,7 +124,7 @@ func validateBody(body hcl.Body, afterMerge bool) error {
 					apiBasePath = path.Join(serverBasePath, apiBasePath)
 					for _, innerInnerBlock := range innerBlock.Body.Blocks {
 						if innerInnerBlock.Type == endpoint {
-							if err = registerEndpointPattern(uniqueEndpoints, apiBasePath, innerInnerBlock); err != nil {
+							if err = registerEndpointPattern(uniqueEndpoints, apiBasePath, innerInnerBlock, afterMerge); err != nil {
 								return err
 							}
 						}
@@ -137,25 +137,50 @@ func validateBody(body hcl.Body, afterMerge bool) error {
 	return nil
 }
 
-func getBasePath(bl *hclsyntax.Block) (string, error) {
+func checkPathSegments(pathType, path string, r hcl.Range) error {
+	for _, segment := range strings.Split(path, "/") {
+		if segment == "." || segment == ".." {
+			return newDiagErr(&r, pathType+` must not contain "." or ".." segments`)
+		}
+	}
+	return nil
+}
+
+func getBasePath(bl *hclsyntax.Block, afterMerge bool) (string, error) {
 	basePath := ""
 	if bp, set := bl.Body.Attributes["base_path"]; set {
 		bpv, diags := bp.Expr.Value(nil)
 		if diags.HasErrors() {
 			return "", diags
 		}
-		if bpv.Type() != cty.String {
-			sr := bp.Expr.StartRange()
-			return "", newDiagErr(&sr, "base_path must evaluate to string")
+		r := bp.Expr.StartRange()
+		if !afterMerge && bpv.Type() != cty.String {
+			return "", newDiagErr(&r, "base_path must evaluate to string")
 		}
 		basePath = bpv.AsString()
+		if !afterMerge {
+			if err := checkPathSegments("base_path", basePath, r); err != nil {
+				return "", err
+			}
+		}
 	}
 
 	return basePath, nil
 }
 
-func registerEndpointPattern(endpointPatterns map[string]struct{}, basePath string, bl *hclsyntax.Block) error {
-	pattern := utils.JoinOpenAPIPath(basePath, bl.Labels[0])
+func registerEndpointPattern(endpointPatterns map[string]struct{}, basePath string, bl *hclsyntax.Block, afterMerge bool) error {
+	pattern := bl.Labels[0]
+	if !afterMerge {
+		if !strings.HasPrefix(pattern, "/") {
+			return newDiagErr(&bl.LabelRanges[0], `endpoint path pattern must start with "/"`)
+		}
+
+		if err := checkPathSegments("endpoint path pattern", pattern, bl.LabelRanges[0]); err != nil {
+			return err
+		}
+	}
+
+	pattern = utils.JoinOpenAPIPath(basePath, pattern)
 	pattern = reCleanPattern.ReplaceAllString(pattern, "{}")
 	if _, set := endpointPatterns[pattern]; set {
 		return newDiagErr(&bl.LabelRanges[0], "duplicate endpoint")


Test patch:
diff --git a/config/configload/load_test.go b/config/configload/load_test.go
index ab486e96..da80be20 100644
--- a/config/configload/load_test.go
+++ b/config/configload/load_test.go
@@ -289,21 +289,16 @@ func TestEndpointPaths(t *testing.T) {
 		expected   string
 	}{
 		{"only /", "", "", "/", "/"},
-		{"missing /", "", "", "path", "/path"},
 		{"simple path", "", "", "/pa/th", "/pa/th"},
 		{"trailing /", "", "", "/pa/th/", "/pa/th/"},
 		{"double /", "", "", "//", "//"},
 		{"double /", "", "", "//path", "//path"},
 		{"double /", "", "", "/pa//th", "/pa//th"},
-		{"/./", "", "", "/./", "/./"},
-		{"/../", "", "", "/../", "/../"},
 
 		{"param", "", "", "/{param}", "/{param}"},
 
 		{"server base_path /", "/", "", "/", "/"},
 		{"server base_path /", "/", "", "/path", "/path"},
-		{"server base_path /", "/", "", "pa/th", "/pa/th"},
-		{"server base_path /", "/", "", "pa/th/", "/pa/th/"},
 		{"server base_path", "/server", "", "/path", "/server/path"},
 		{"server base_path with / endpoint", "/server", "", "/", "/server"},
 		{"server base_path missing /", "server", "", "/path", "/server/path"},
@@ -313,8 +308,6 @@ func TestEndpointPaths(t *testing.T) {
 
 		{"api base_path /", "", "/", "/", "/"},
 		{"api base_path /", "", "/", "/path", "/path"},
-		{"api base_path /", "", "/", "pa/th", "/pa/th"},
-		{"api base_path /", "", "/", "pa/th/", "/pa/th/"},
 		{"api base_path", "", "/api", "/path", "/api/path"},
 		{"api base_path with / endpoint", "", "/api", "/", "/api"},
 		{"api base_path missing /", "", "api", "/path", "/api/path"},
diff --git a/config/configload/validate_test.go b/config/configload/validate_test.go
index 9023c73d..2d2979f5 100644
--- a/config/configload/validate_test.go
+++ b/config/configload/validate_test.go
@@ -630,6 +630,212 @@ func Test_validateBody(t *testing.T) {
 			 }`,
 			"",
 		},
+		{
+			"invalid server base_path pattern single dot 1",
+			`server {
+			   base_path = "./s"
+			 }`,
+			`couper.hcl:2,20-23: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern single dot 2",
+			`server {
+			   base_path = "/./s"
+			 }`,
+			`couper.hcl:2,20-24: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern single dot 3",
+			`server {
+			   base_path = "/s/./s"
+			 }`,
+			`couper.hcl:2,20-26: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern single dot 4",
+			`server {
+			   base_path = "/s/."
+			 }`,
+			`couper.hcl:2,20-24: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern double dot 1",
+			`server {
+			   base_path = "../s"
+			 }`,
+			`couper.hcl:2,20-24: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern double dot 2",
+			`server {
+			   base_path = "/../s"
+			 }`,
+			`couper.hcl:2,20-25: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern double dot 3",
+			`server {
+			   base_path = "/s/../s"
+			 }`,
+			`couper.hcl:2,20-27: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid server base_path pattern double dot 4",
+			`server {
+			   base_path = "/s/.."
+			 }`,
+			`couper.hcl:2,20-25: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern single dot 1",
+			`server {
+			   api {
+			     base_path = "./s"
+			   }
+			 }`,
+			`couper.hcl:3,22-25: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern single dot 2",
+			`server {
+			   api {
+			     base_path = "/./s"
+			   }
+			 }`,
+			`couper.hcl:3,22-26: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern single dot 3",
+			`server {
+			   api {
+			     base_path = "/s/./s"
+			   }
+			 }`,
+			`couper.hcl:3,22-28: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern single dot 4",
+			`server {
+			   api {
+			     base_path = "/s/."
+			   }
+			 }`,
+			`couper.hcl:3,22-26: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern double dot 1",
+			`server {
+			   api {
+			     base_path = "../s"
+			   }
+			 }`,
+			`couper.hcl:3,22-26: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern double dot 2",
+			`server {
+			   api {
+			     base_path = "/../s"
+			   }
+			 }`,
+			`couper.hcl:3,22-27: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern double dot 3",
+			`server {
+			   api {
+			     base_path = "/s/../s"
+			   }
+			 }`,
+			`couper.hcl:3,22-29: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid api base_path pattern double dot 4",
+			`server {
+			   api {
+			     base_path = "/s/.."
+			   }
+			 }`,
+			`couper.hcl:3,22-27: base_path must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid endpoint pattern single dot 1",
+			`server {
+			   api {
+			     endpoint "./foo" {
+			       response {
+			         body = "1"
+			       }
+			     }
+			   }
+			 }`,
+			`couper.hcl:3,18-25: endpoint path pattern must start with "/"; `,
+		},
+		{
+			"invalid endpoint pattern single dot 2",
+			`server {
+			   api {
+			     endpoint "/foo/./bar" {
+			       response {
+			         body = "1"
+			       }
+			     }
+			   }
+			 }`,
+			`couper.hcl:3,18-30: endpoint path pattern must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid endpoint pattern single dot 3",
+			`server {
+			   api {
+			     endpoint "/foo/." {
+			       response {
+			         body = "1"
+			       }
+			     }
+			   }
+			 }`,
+			`couper.hcl:3,18-26: endpoint path pattern must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid endpoint pattern double dot 1",
+			`server {
+			   api {
+			     endpoint "../foo" {
+			       response {
+			         body = "1"
+			       }
+			     }
+			   }
+			 }`,
+			`couper.hcl:3,18-26: endpoint path pattern must start with "/"; `,
+		},
+		{
+			"invalid endpoint pattern double dot 2",
+			`server {
+			   api {
+			     endpoint "/foo/../bar" {
+			       response {
+			         body = "1"
+			       }
+			     }
+			   }
+			 }`,
+			`couper.hcl:3,18-31: endpoint path pattern must not contain "." or ".." segments; `,
+		},
+		{
+			"invalid endpoint pattern double dot 3",
+			`server {
+			   api {
+			     endpoint "/foo/.." {
+			       response {
+			         body = "1"
+			       }
+			     }
+			   }
+			 }`,
+			`couper.hcl:3,18-27: endpoint path pattern must not contain "." or ".." segments; `,
+		},
 		{
 			"duplicate endpoint pattern 1",
 			`server {