CVE-2020-26294: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: Vela is a Pipeline Automation (CI/CD) framework built on Linux container technology written in Golang. In Vela compiler before version 0.6.1 there is a vulnerability which allows exposure of server configuration. It impacts all users of Vela. An attacker can use Sprig's `env` function to retrieve configuration information, see referenced GHSA for an example. This has been fixed in version 0.6.1. In addition to upgrading, it is recommended to rotate all secrets.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 5.3
Published: 2021-01-04T19:15Z
Repository: https://github.com/go-vela/compiler
Language: Go
Files changed: template/native/render.go, template/native/render_test.go, template/native/testdata/disallowed/tmpl_env.yml, template/native/testdata/disallowed/tmpl_expandenv.yml
Commit date: 2020-12-29 14:50:15 -0600
Commit message: fix: disallow functions (#93)


Diff: diff --git a/template/native/render.go b/template/native/render.go
index 4683db6..7f723af 100644
--- a/template/native/render.go
+++ b/template/native/render.go
@@ -19,15 +19,22 @@ func Render(tmpl string, s *types.Step) (types.StepSlice, error) {
 
 	velaFuncs := funcHandler{envs: convertPlatformVars(s.Environment)}
 	templateFuncMap := map[string]interface{}{
 		"vela": velaFuncs.returnPlatformVar,
 	}
+	// modify Masterminds/sprig functions
+	// to remove OS functions
+	//
+	// https://masterminds.github.io/sprig/os.html
+	sf := sprig.TxtFuncMap()
+	delete(sf, "env")
+	delete(sf, "expandenv")
 
 	// parse the template with Masterminds/sprig functions
 	//
 	// https://pkg.go.dev/github.com/Masterminds/sprig?tab=doc#TxtFuncMap
-	t, err := template.New(s.Name).Funcs(sprig.TxtFuncMap()).Funcs(templateFuncMap).Parse(tmpl)
+	t, err := template.New(s.Name).Funcs(sf).Funcs(templateFuncMap).Parse(tmpl)
 	if err != nil {
 		return types.StepSlice{}, fmt.Errorf("unable to parse template %s: %v", s.Template.Name, err)
 	}
 
 	// apply the variables to the parsed template
diff --git a/template/native/render_test.go b/template/native/render_test.go
index b6f4ded..941624d 100644
--- a/template/native/render_test.go
+++ b/template/native/render_test.go
@@ -4,10 +4,11 @@
 
 package native
 
 import (
 	"io/ioutil"
+	"reflect"
 	"testing"
 
 	"github.com/go-vela/types/raw"
 	"github.com/google/go-cmp/cmp"
 
@@ -80,5 +81,45 @@ func TestNative_Render(t *testing.T) {
 				}
 			}
 		})
 	}
 }
+
+func TestNative_Render_DisallowedFunc_Env(t *testing.T) {
+	// setup types
+	want := yaml.StepSlice{}
+
+	// run test
+	tmpl, err := ioutil.ReadFile("testdata/disallowed/tmpl_env.yml")
+	if err != nil {
+		t.Errorf("Reading file returned err: %v", err)
+	}
+
+	got, err := Render(string(tmpl), &yaml.Step{})
+	if err == nil {
+		t.Errorf("Render should have returned err")
+	}
+
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Render is %v, want %v", got, want)
+	}
+}
+
+func TestNative_Render_DisallowedFunc_ExpandEnv(t *testing.T) {
+	// setup types
+	want := yaml.StepSlice{}
+
+	// run test
+	tmpl, err := ioutil.ReadFile("testdata/disallowed/tmpl_expandenv.yml")
+	if err != nil {
+		t.Errorf("Reading file returned err: %v", err)
+	}
+
+	got, err := Render(string(tmpl), &yaml.Step{})
+	if err == nil {
+		t.Errorf("Render should have returned err")
+	}
+
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Render is %v, want %v", got, want)
+	}
+}
diff --git a/template/native/testdata/disallowed/tmpl_env.yml b/template/native/testdata/disallowed/tmpl_env.yml
new file mode 100644
index 0000000..c6eb1aa
--- /dev/null
+++ b/template/native/testdata/disallowed/tmpl_env.yml
@@ -0,0 +1,8 @@
+metadata:
+  template: true
+
+steps:
+  - name: echo
+    commands:
+      - echo {{ env "VELA_SOURCE_CLIENT" }}
+    image: alpine:latest
\ No newline at end of file
diff --git a/template/native/testdata/disallowed/tmpl_expandenv.yml b/template/native/testdata/disallowed/tmpl_expandenv.yml
new file mode 100644
index 0000000..3c0e390
--- /dev/null
+++ b/template/native/testdata/disallowed/tmpl_expandenv.yml
@@ -0,0 +1,8 @@
+metadata:
+  template: true
+
+steps:
+  - name: echo
+    commands:
+      - echo {{ expandenv "Your client id is set to $VELA_SOURCE_CLIENT" }}
+    image: alpine:latest
\ No newline at end of file

Vulnerable code: 	t, err := template.New(s.Name).Funcs(sprig.TxtFuncMap()).Funcs(templateFuncMap).Parse(tmpl)
Fixed code: 	// modify Masterminds/sprig functions
	// to remove OS functions
	//
	// https://masterminds.github.io/sprig/os.html
	sf := sprig.TxtFuncMap()
	delete(sf, "env")
	delete(sf, "expandenv")
	t, err := template.New(s.Name).Funcs(sf).Funcs(templateFuncMap).Parse(tmpl)
	"reflect"

func TestNative_Render_DisallowedFunc_Env(t *testing.T) {
	// setup types
	want := yaml.StepSlice{}

	// run test
	tmpl, err := ioutil.ReadFile("testdata/disallowed/tmpl_env.yml")
	if err != nil {
		t.Errorf("Reading file returned err: %v", err)
	}

	got, err := Render(string(tmpl), &yaml.Step{})
	if err == nil {
		t.Errorf("Render should have returned err")
	}

	if !reflect.DeepEqual(got, want) {
		t.Errorf("Render is %v, want %v", got, want)
	}
}

func TestNative_Render_DisallowedFunc_ExpandEnv(t *testing.T) {
	// setup types
	want := yaml.StepSlice{}

	// run test
	tmpl, err := ioutil.ReadFile("testdata/disallowed/tmpl_expandenv.yml")
	if err != nil {
		t.Errorf("Reading file returned err: %v", err)
	}

	got, err := Render(string(tmpl), &yaml.Step{})
	if err == nil {
		t.Errorf("Render should have returned err")
	}

	if !reflect.DeepEqual(got, want) {
		t.Errorf("Render is %v, want %v", got, want)
	}
}
metadata:
  template: true

steps:
  - name: echo
    commands:
      - echo {{ env "VELA_SOURCE_CLIENT" }}
    image: alpine:latest
metadata:
  template: true

steps:
  - name: echo
    commands:
      - echo {{ expandenv "Your client id is set to $VELA_SOURCE_CLIENT" }}
    image: alpine:latest