From be8dbb426ea79b55d29b18066d5610ffb1c9677d Mon Sep 17 00:00:00 2001
From: Zachary Wasserman <zach@dactiv.llc>
Date: Wed, 22 Apr 2020 13:59:40 -0700
Subject: [PATCH] Sanitize column names for order key in MySQL (#2224)

This adds a SQL injection prevention for a case in which we cannot use
parameters in the query.

It is not clear that this was possible to exploit. If it was possible,
it would have required a valid login to the Fleet server.
---
 server/datastore/mysql/mysql.go      | 12 +++++++++++-
 server/datastore/mysql/mysql_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 server/datastore/mysql/mysql_test.go

diff --git a/server/datastore/mysql/mysql.go b/server/datastore/mysql/mysql.go
index ab66795d2e..0c20ec53cc 100644
--- a/server/datastore/mysql/mysql.go
+++ b/server/datastore/mysql/mysql.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/url"
+	"regexp"
 	"time"
 
 	"github.com/WatchBeam/clock"
@@ -25,6 +26,10 @@ const (
 	defaultSelectLimit = 100000
 )
 
+var (
+	columnCharsRegexp = regexp.MustCompile(`[^\w-]`)
+)
+
 // Datastore is an implementation of kolide.Datastore interface backed by
 // MySQL
 type Datastore struct {
@@ -249,14 +254,19 @@ func (d *Datastore) log(msg string) {
 	d.logger.Log("comp", d.Name(), "msg", msg)
 }
 
+func sanitizeColumn(col string) string {
+	return columnCharsRegexp.ReplaceAllString(col, "")
+}
+
 func appendListOptionsToSQL(sql string, opts kolide.ListOptions) string {
 	if opts.OrderKey != "" {
 		direction := "ASC"
 		if opts.OrderDirection == kolide.OrderDescending {
 			direction = "DESC"
 		}
+		orderKey := sanitizeColumn(opts.OrderKey)
 
-		sql = fmt.Sprintf("%s ORDER BY %s %s", sql, opts.OrderKey, direction)
+		sql = fmt.Sprintf("%s ORDER BY %s %s", sql, orderKey, direction)
 	}
 	// REVIEW: If caller doesn't supply a limit apply a default limit of 1000
 	// to insure that an unbounded query with many results doesn't consume too
diff --git a/server/datastore/mysql/mysql_test.go b/server/datastore/mysql/mysql_test.go
new file mode 100644
index 0000000000..baa93894b4
--- /dev/null
+++ b/server/datastore/mysql/mysql_test.go
@@ -0,0 +1,26 @@
+package mysql
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSanitizeColumn(t *testing.T) {
+	testCases := []struct {
+		input  string
+		output string
+	}{
+		{"foobar-column", "foobar-column"},
+		{"foobar_column", "foobar_column"},
+		{"foobar;column", "foobarcolumn"},
+		{"foobar#", "foobar"},
+		{"foobar*baz", "foobarbaz"},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.input, func(t *testing.T) {
+			assert.Equal(t, tt.output, sanitizeColumn(tt.input))
+		})
+	}
+}