From 776f2c0a2d9e768cd9225d0834f54f9fc0ec9391 Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Mon, 12 Sep 2022 16:37:33 -0400
Subject: [PATCH] variables: restrict allowed paths for variables (#14547)

Restrict variable paths to RFC3986 URL-safe characters that don't conflict with
the use of characters "@" and "." in `template` blocks. This prevents users from
writing variables that will require tricky templating syntax or that they simply
won't be able to use.

Also restrict the length so that a user can't make queries in the state store
unusually expensive (as they are O(k) on the key length).
---
 nomad/structs/variables.go      | 26 ++++++++++++++++++++------
 nomad/structs/variables_test.go |  3 +++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/nomad/structs/variables.go b/nomad/structs/variables.go
index 4f7045a3f..8c7260d1a 100644
--- a/nomad/structs/variables.go
+++ b/nomad/structs/variables.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"regexp"
 	"strings"
 )
 
@@ -145,12 +146,25 @@ func (sv VariableData) Copy() VariableData {
 	}
 }
 
-func (sv VariableDecrypted) Validate() error {
+var (
+	// validVariablePath is used to validate a variable path. We restrict to
+	// RFC3986 URL-safe characters that don't conflict with the use of
+	// characters "@" and "." in template blocks. We also restrict the length so
+	// that a user can't make queries in the state store unusually expensive (as
+	// they are O(k) on the key length)
+	validVariablePath = regexp.MustCompile("^[a-zA-Z0-9-_~/]{1,128}$")
+)
 
-	if len(sv.Path) == 0 {
+func (v VariableDecrypted) Validate() error {
+
+	if len(v.Path) == 0 {
 		return fmt.Errorf("variable requires path")
 	}
-	parts := strings.Split(sv.Path, "/")
+	if !validVariablePath.MatchString(v.Path) {
+		return fmt.Errorf("invalid path %q", v.Path)
+	}
+
+	parts := strings.Split(v.Path, "/")
 	switch {
 	case len(parts) == 1 && parts[0] == "nomad":
 		return fmt.Errorf("\"nomad\" is a reserved top-level directory path, but you may write variables to \"nomad/jobs\" or below")
@@ -158,13 +172,13 @@ func (sv VariableDecrypted) Validate() error {
 		return fmt.Errorf("only paths at \"nomad/jobs\" or below are valid paths under the top-level \"nomad\" directory")
 	}
 
-	if len(sv.Items) == 0 {
+	if len(v.Items) == 0 {
 		return errors.New("empty variables are invalid")
 	}
-	if sv.Items.Size() > maxVariableSize {
+	if v.Items.Size() > maxVariableSize {
 		return errors.New("variables are limited to 16KiB in total size")
 	}
-	if sv.Namespace == AllNamespacesSentinel {
+	if v.Namespace == AllNamespacesSentinel {
 		return errors.New("can not target wildcard (\"*\")namespace")
 	}
 	return nil
diff --git a/nomad/structs/variables_test.go b/nomad/structs/variables_test.go
index 20eb1cd04..95035ef42 100644
--- a/nomad/structs/variables_test.go
+++ b/nomad/structs/variables_test.go
@@ -51,6 +51,9 @@ func TestStructs_VariableDecrypted_Validate(t *testing.T) {
 		{path: "nomad/jobs", ok: true},
 		{path: "nomadjobs", ok: true},
 		{path: "nomad/jobs/whatever", ok: true},
+		{path: "example/_-~/whatever", ok: true},
+		{path: "example/@whatever"},
+		{path: "example/what.ever"},
 	}
 	for _, tc := range testCases {
 		tc := tc