Bugfix: Use of sqlx library resulted in sqlite handle leaks (Velocide…

…x#3675)

This causes excessive memory use. Also because connections are held
open it was impossible to remove the files on windows causing tmp file
leaks.

---------

Co-authored-by: snyk-bot <[email protected]>

artifacts/definitions/Server/Monitor/Health.yaml

@@ -27,7 +27,7 @@ reports:
       {{ define "CPU" }}
           SELECT _ts as Timestamp,
               CPUPercent,
-              MemoryUse / 1048576 AS MemoryUse_Mb,
+              int(int=MemoryUse / 1048576) AS MemoryUse_Mb,
               TotalFrontends
           FROM source(source="Prometheus",
                       start_time=StartTime, end_time=EndTime,

bin/artifacts.go

@@ -24,6 +24,7 @@ import (
 	"regexp"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/Velocidex/ordereddict"
 	"github.com/Velocidex/yaml/v2"
@@ -221,6 +222,18 @@ func doArtifactCollect() error {
 		sm.Wg.Done()
 	})
 
+	// If interrupt has occured we cancel everything and wait for any
+	// cleanups to occur. If we return too quickly from the main
+	// thread, we might leave some tempfiles behind.
+	defer func() {
+		select {
+		case <-ctx.Done():
+			scope.Log("ERROR:Interrupted! Sleeping on exit to allow cleanup")
+			time.Sleep(time.Second)
+		default:
+		}
+	}()
+
 	if *artifact_command_collect_hardmemory > 0 {
 		scope.Log("Installing hard memory limit of %v bytes",
 			*artifact_command_collect_hardmemory)

gui/velociraptor/package.json

@@ -13,7 +13,7 @@
         "@fortawesome/free-solid-svg-icons": "^6.6.0",
         "@fortawesome/react-fontawesome": "0.2.2",
         "@popperjs/core": "^2.11.8",
-        "ace-builds": "1.35.2",
+        "ace-builds": "1.35.3",
         "axios": ">=1.7.2",
         "axios-retry": "3.9.1",
         "bootstrap": "5.3.3",

result_sets/simple/fixtures/TestTransformFilter.golden

@@ -1,22 +1,22 @@
 {
  "Filtered by Str Sort Asc": [
-  {
-   "Str": "Another string",
-   "Int": 12
-  },
   {
    "Str": "This is a string",
    "Int": 6
+  },
+  {
+   "Str": "Another string",
+   "Int": 12
   }
  ],
  "Filtered out by Str Sort Asc": [
-  {
-   "Str": null,
-   "Int": -10
-  },
   {
    "Str": "Hello world",
    "Int": 12
+  },
+  {
+   "Str": null,
+   "Int": -10
   }
  ],
  "Filtered by Str - nil": [
@@ -27,16 +27,16 @@
  ],
  "Filtered by Int Sort Asc": [
   {
-   "Str": null,
-   "Int": -10
+   "Str": "Hello world",
+   "Int": 12
   },
   {
    "Str": "Another string",
    "Int": 12
   },
   {
-   "Str": "Hello world",
-   "Int": 12
+   "Str": null,
+   "Int": -10
   }
  ]
 }

result_sets/simple/fixtures/TestTransformed.golden

@@ -1,52 +1,52 @@
 {
  "Stacked by Bar": [
   {
-   "value": "Bar0.txt",
+   "value": "Bar9.txt",
    "idx": 0,
    "c": 5
   },
   {
-   "value": "Bar1.txt",
+   "value": "Bar8.txt",
    "idx": 5,
    "c": 5
   },
   {
-   "value": "Bar2.txt",
+   "value": "Bar7.txt",
    "idx": 10,
    "c": 5
   },
   {
-   "value": "Bar3.txt",
+   "value": "Bar6.txt",
    "idx": 15,
    "c": 5
   },
   {
-   "value": "Bar4.txt",
+   "value": "Bar5.txt",
    "idx": 20,
    "c": 5
   },
   {
-   "value": "Bar5.txt",
+   "value": "Bar4.txt",
    "idx": 25,
    "c": 5
   },
   {
-   "value": "Bar6.txt",
+   "value": "Bar3.txt",
    "idx": 30,
    "c": 5
   },
   {
-   "value": "Bar7.txt",
+   "value": "Bar2.txt",
    "idx": 35,
    "c": 5
   },
   {
-   "value": "Bar8.txt",
+   "value": "Bar1.txt",
    "idx": 40,
    "c": 5
   },
   {
-   "value": "Bar9.txt",
+   "value": "Bar0.txt",
    "idx": 45,
    "c": 5
   }

result_sets/simple/transformed.go

@@ -210,7 +210,7 @@ func (self ResultSetFactory) getSortedReader(
 		file_store_factory, self,
 		sorter.MergeSorter{10000}.Sort(
 			ctx, scope, sorter_input_chan,
-			options.SortColumn, !options.SortAsc),
+			options.SortColumn, options.SortAsc),
 		options.SortColumn)
 	if err != nil {
 		return nil, err

vql/parsers/sql.go → vql/parsers/sql/sql.go

@@ -1,7 +1,8 @@
-package parsers
+package sql
 
 import (
 	"context"
+	"database/sql"
 	"errors"
 	"fmt"
 	"reflect"
@@ -10,7 +11,6 @@ import (
 
 	"github.com/Velocidex/ordereddict"
 	_ "github.com/go-sql-driver/mysql"
-	"github.com/jmoiron/sqlx"
 	_ "github.com/lib/pq"
 	"www.velocidex.com/golang/velociraptor/accessors"
 	"www.velocidex.com/golang/velociraptor/acls"
@@ -37,7 +37,10 @@ type SQLPluginArgs struct {
 type SQLPlugin struct{}
 
 // Get DB handle from cache if it exists, else create a new connection
-func (self SQLPlugin) GetHandleOther(scope vfilter.Scope, connstring string, driver string) (*sqlx.DB, error) {
+func (self SQLPlugin) GetHandleOther(
+	scope vfilter.Scope, connstring string,
+	driver string) (*DB, error) {
+
 	if connstring == "" {
 		return nil, fmt.Errorf("file parameter required for %s driver!", driver)
 	}
@@ -46,7 +49,7 @@ func (self SQLPlugin) GetHandleOther(scope vfilter.Scope, connstring string, dri
 	client := vql_subsystem.CacheGet(scope, cacheKey)
 
 	if utils.IsNil(client) {
-		client, err := sqlx.Open(driver, connstring)
+		client, err := sql.Open(driver, connstring)
 		if err != nil {
 			// Cache failure to connect.
 			vql_subsystem.CacheSet(scope, cacheKey, err)
@@ -63,20 +66,23 @@ func (self SQLPlugin) GetHandleOther(scope vfilter.Scope, connstring string, dri
 		err = vql_subsystem.GetRootScope(scope).AddDestructor(func() {
 			client.Close()
 		})
+
 		if err != nil {
 			client.Close()
 			return nil, err
 		}
 
 		vql_subsystem.CacheSet(scope, cacheKey, client)
-		return client, nil
+		return &DB{DB: client}, nil
 
 	}
+
+	// Cache errors to avoid making lots of bad connections.
 	switch t := client.(type) {
 	case error:
 		return nil, t
 
-	case *sqlx.DB:
+	case *DB:
 		return t, nil
 
 	default:
@@ -106,7 +112,7 @@ func (self SQLPlugin) Call(
 			arg.Accessor = "file"
 		}
 
-		var handle *sqlx.DB
+		var handle *DB
 
 		switch arg.Driver {
 		default:
@@ -138,6 +144,8 @@ func (self SQLPlugin) Call(
 			}
 		}
 
+		defer handle.Close()
+
 		query_parameters := []interface{}{}
 		if arg.Args != nil {
 			args_value := reflect.Indirect(reflect.ValueOf(arg.Args))
@@ -155,31 +163,47 @@ func (self SQLPlugin) Call(
 		if query == "" {
 			return
 		}
-		rows, err := handle.Queryx(query, query_parameters...)
+		rows, err := handle.Query(query, query_parameters...)
 		if err != nil {
 			scope.Log("sql: %v", err)
 			return
 		}
 		defer rows.Close()
+
 		columns, err := rows.Columns()
 		if err != nil {
-			scope.Log("sql: %s", err)
+			scope.Log("sql: %v", err)
 		}
+
 		for rows.Next() {
-			row := ordereddict.NewDict()
-			values, err := rows.SliceScan()
+			// Create a slice of interface{}'s to represent each
+			// column, and a second slice to contain pointers to each
+			// item in the columns slice.
+			row_values := make([]interface{}, len(columns))
+			row_pointers := make([]interface{}, len(columns))
+			for i, _ := range columns {
+				row_pointers[i] = &row_values[i]
+			}
+
+			// Scan the result into the column pointers...
+			err = rows.Scan(row_pointers...)
 			if err != nil {
 				scope.Log("sql: %v", err)
 				return
 			}
 
-			for idx, item := range columns {
-				var value interface{} = values[idx]
+			row := ordereddict.NewDict()
+			for i, name := range columns {
+				value_pointer := row_pointers[i].(*interface{})
+				value := *value_pointer
+
+				// Special case raw []bytes to be strings.
 				bytes_value, ok := value.([]byte)
 				if ok {
 					value = string(bytes_value)
 				}
-				row.Set(item, value)
+
+				row.Set(name, value)
 			}
 
 			select {