scottk/vault
1
0
Fork 0
mirror of https://github.com/optim-enterprises-bv/vault.git synced 2025-11-01 19:17:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use parameters when executing prepared statements rather than fmt.Sprintf (#9013)

Browse Source 
* Don't use string formatting to prepare queries.

We should, when possible, use the built-in params and ? format when
preparing and executing a query. This is done to prevent SQL Injection
attacks.

* Revert some changes due to failing tests, update mssql go driver

* Add docker container startup for some MSSQL tests

* Remove acceptance test flagging, add more SQL injection protection

* Refactor MSSQL prepareTestContainer to a test helper

Also, remove all ? references and convert them to @p*
This commit is contained in:
Lauren Voswinkel
2020-05-21 16:07:18 -07:00
committed by GitHub
parent 87f47c216c
commit 1a322e2026
7 changed files with 84 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
plugins/database/mssql/mssql.go
View File

@@ -215,14 +215,14 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error {
// sessions. There cannot be any active sessions before we drop the logins
// This isn't done in a transaction because even if we fail along the way,
// we want to remove as much access as possible
sessionStmt, err := db.PrepareContext(ctx, fmt.Sprintf(
"SELECT session_id FROM sys.dm_exec_sessions WHERE login_name = '%s';", username))
sessionStmt, err := db.PrepareContext(ctx,
"SELECT session_id FROM sys.dm_exec_sessions WHERE login_name = @p1;")
if err != nil {
return err
}
defer sessionStmt.Close()
sessionRows, err := sessionStmt.QueryContext(ctx)
sessionRows, err := sessionStmt.QueryContext(ctx, username)
if err != nil {
return err
}
@@ -243,13 +243,13 @@ func (m *MSSQL) revokeUserDefault(ctx context.Context, username string) error {
// we need to drop the database users before we can drop the login and the role
// This isn't done in a transaction because even if we fail along the way,
// we want to remove as much access as possible
stmt, err := db.PrepareContext(ctx, fmt.Sprintf("EXEC master.dbo.sp_msloginmappings '%s';", username))
stmt, err := db.PrepareContext(ctx, "EXEC master.dbo.sp_msloginmappings @p1;")
if err != nil {
return err
}
defer stmt.Close()
rows, err := stmt.QueryContext(ctx)
rows, err := stmt.QueryContext(ctx, username)
if err != nil {
return err
}

26
plugins/database/mssql/mssql_test.go
View File

@@ -4,19 +4,17 @@ import (
"context"
"database/sql"
"fmt"
"os"
"strings"
"testing"
"time"
mssqlhelper "github.com/hashicorp/vault/helper/testhelpers/mssql"
"github.com/hashicorp/vault/sdk/database/dbplugin"
)
func TestMSSQL_Initialize(t *testing.T) {
if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
t.SkipNow()
}
connURL := os.Getenv("MSSQL_URL")
cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
defer cleanup()
connectionDetails := map[string]interface{}{
"connection_url": connURL,
@@ -50,10 +48,8 @@ func TestMSSQL_Initialize(t *testing.T) {
}
func TestMSSQL_CreateUser(t *testing.T) {
if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
t.SkipNow()
}
connURL := os.Getenv("MSSQL_URL")
cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
defer cleanup()
connectionDetails := map[string]interface{}{
"connection_url": connURL,
@@ -91,10 +87,8 @@ func TestMSSQL_CreateUser(t *testing.T) {
}
func TestMSSQL_RotateRootCredentials(t *testing.T) {
if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
t.SkipNow()
}
connURL := os.Getenv("MSSQL_URL")
cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
defer cleanup()
connectionDetails := map[string]interface{}{
"connection_url": connURL,
@@ -130,10 +124,8 @@ func TestMSSQL_RotateRootCredentials(t *testing.T) {
}
func TestMSSQL_RevokeUser(t *testing.T) {
if os.Getenv("MSSQL_URL") == "" || os.Getenv("VAULT_ACC") != "1" {
t.SkipNow()
}
connURL := os.Getenv("MSSQL_URL")
cleanup, connURL := mssqlhelper.PrepareMSSQLTestContainer(t)
defer cleanup()
connectionDetails := map[string]interface{}{
"connection_url": connURL,