fix: libddwaf does not accept nullptr-valued blank strings (#120)

Inadvertently replaced the pointer to a `\x00` byte that was used when
encoding a blank string with a `0` (`null`) pointer; which `libddwaf`
does not currently accept.

This commit restores the previous encoding behavior, and adds a test
that was created based on the ruleset that allowed discovery of this
issue.

---

Also fixes up the CGO-exported symbol name to be at V4 so it does not
clash with the one exported by V3.

Author: RomainMuller

README.md

@@ -143,3 +143,12 @@ Another requirement of `libddwaf` is to have a FHS filesystem on your machine an
 - Do not use `uintptr` as function arguments or results types, coming from `unsafe.Pointer` casts of Go values, because they escape the pointer analysis which can create wrongly optimized code and crash. Pointer arithmetic is of course necessary in such a library but must be kept in the same function scope.
 - GDB is available on arm64 but is not officially supported so it usually crashes pretty fast (as of June 2023)
 - No pointer to variables on the stack shall be sent to the C code because Go stacks can be moved during the C call. More on this [here](https://medium.com/@trinad536/escape-analysis-in-golang-fc81b78f3550)
+
+## Debugging
+
+Debug-logging can be enabled for underlying C/C++ library by building (or testing) by setting the
+`DD_APPSEC_WAF_LOG_LEVEL` environment variable to one of: `trace`, `debug`, `info`, `warn` (or
+`warning`), `error`, `off` (which is the default behavior and logs nothing).
+
+The `DD_APPSEC_WAF_LOG_FILTER` environment variable can be set to a valid (per the `regexp` package)
+regular expression to limit logging to only messages that match the regular expression.

builder_test.go

@@ -8,20 +8,20 @@
 package libddwaf
 
 import (
+	"bytes"
 	"encoding/json"
 	"maps"
 	"net/http"
 	"os"
 	"testing"
+	"time"
 
-	"github.com/DataDog/go-libddwaf/v4/internal/log"
 	"github.com/DataDog/go-libddwaf/v4/timer"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestBuilder(t *testing.T) {
-	wafLib.SetLogCb(log.CallbackFunctionPointer(), log.LevelDebug)
-
 	if supported, err := Usable(); !supported || err != nil {
 		t.Skipf("target is not supported by the WAF: %v", err)
 	}
@@ -299,4 +299,89 @@ func TestBuilder(t *testing.T) {
 		require.NotNil(t, handle)
 		handle.Close()
 	})
+
+	t.Run("blank-string-encoding", func(t *testing.T) {
+		var rulesJSON = `{
+			"version": "2.1",
+			"metadata": {
+				"rules_version": "1.2.6"
+			},
+			"rules": [
+				{
+					"id": "canary_rule4",
+					"name": "Canary 4",
+					"tags": {
+						"type": "security_scanner",
+						"category": "attack_attempt"
+					},
+					"conditions": [
+						{
+							"parameters": {
+								"inputs": [
+									{
+										"address": "server.request.headers.no_cookies",
+										"key_path": [
+											"user-agent"
+										]
+									}
+								],
+								"regex": "^Canary\\/v4"
+							},
+							"operator": "match_regex"
+						}
+					],
+					"on_match": [
+						"block4"
+					]
+				}
+			],
+			"actions": [
+				{
+					"id": "block4",
+					"type": "redirect_request",
+					"parameters": {
+						"status_code": 303,
+						"location": ""
+					}
+				}
+			]
+		}
+		`
+
+		builder, err := NewBuilder("", "")
+		require.NoError(t, err)
+
+		dec := json.NewDecoder(bytes.NewReader([]byte(rulesJSON)))
+		dec.UseNumber()
+
+		var rules map[string]any
+		require.NoError(t, dec.Decode(&rules))
+
+		diag, err := builder.AddOrUpdateConfig("/", rules)
+		require.NoError(t, err)
+		diag.EachFeature(func(name string, feat *Feature) {
+			assert.Empty(t, feat.Error, "feature %s has top-level error", name)
+			assert.Empty(t, feat.Errors, "feature %s has errors", name)
+			assert.Empty(t, feat.Warnings, "feature %s has warnings", name)
+		})
+
+		waf := builder.Build()
+		require.NotNil(t, waf)
+		defer waf.Close()
+
+		ctx, err := waf.NewContext(time.Hour)
+		require.NoError(t, err)
+		defer ctx.Close()
+
+		res, err := ctx.Run(RunAddressData{
+			Persistent: map[string]any{
+				"server.request.headers.no_cookies": map[string][]string{
+					"user-agent": {"Canary/v4 bazinga"},
+				},
+			},
+		})
+		require.NoError(t, err)
+		assert.NotEmpty(t, res.Events)
+		assert.NotEmpty(t, res.Actions)
+	})
 }

encoder.go

@@ -7,6 +7,7 @@ package libddwaf
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"math"
 	"reflect"
@@ -143,6 +144,8 @@ var nullableTypeKinds = map[reflect.Kind]struct{}{
 	reflect.Chan:          {},
 }
 
+var jsonNumberType = reflect.TypeOf(json.Number(""))
+
 // isValueNil check if the value is nullable and if it is actually nil
 // we cannot directly use value.IsNil() because it panics on non-pointer values
 func isValueNil(value reflect.Value) bool {
@@ -189,6 +192,10 @@ func (encoder *encoder) encode(value reflect.Value, obj *bindings.WAFObject, dep
 	case value.CanFloat(): // any float type or alias
 		encodeNative(unsafe.NativeToUintptr(value.Float()), bindings.WAFFloatType, obj)
 
+	// 		json.Number -- string-represented arbitrary precision numbers
+	case value.Type() == jsonNumberType:
+		encoder.encodeJSONNumber(value.Interface().(json.Number), obj)
+
 	//		Strings
 	case kind == reflect.String: // string type
 		encoder.encodeString(value.String(), obj)
@@ -221,6 +228,23 @@ func (encoder *encoder) encode(value reflect.Value, obj *bindings.WAFObject, dep
 	return nil
 }
 
+func (encoder *encoder) encodeJSONNumber(num json.Number, obj *bindings.WAFObject) {
+	// Important to attempt int64 first, as this is lossless. Values that are either too small or too
+	// large to be represented as int64 can be represented as float64, but this can be lossy.
+	if i, err := num.Int64(); err == nil {
+		encodeNative(uintptr(i), bindings.WAFIntType, obj)
+		return
+	}
+
+	if f, err := num.Float64(); err == nil {
+		encodeNative(unsafe.NativeToUintptr(f), bindings.WAFFloatType, obj)
+		return
+	}
+
+	// Could not store as int64 nor float, so we'll store it as a string...
+	encoder.encodeString(num.String(), obj)
+}
+
 func (encoder *encoder) encodeString(str string, obj *bindings.WAFObject) {
 	size := len(str)
 	if size > encoder.stringMaxSize {

encoder_decoder_test.go

@@ -10,9 +10,11 @@ package libddwaf
 import (
 	"context"
 	"encoding/json"
+	"math"
 	"reflect"
 	"runtime"
 	"sort"
+	"strconv"
 	"testing"
 	"time"
 
@@ -248,6 +250,26 @@ func TestEncodeDecode(t *testing.T) {
 			Input:  map[any]any{"k1": uint64(1), new(string): "string pointer key", "k2": "2"},
 			Output: map[string]any{"k1": uint64(1), "": "string pointer key", "k2": "2"},
 		},
+		{
+			Name:   "json-number.int",
+			Input:  json.Number("1234567890"),
+			Output: int64(1234567890),
+		},
+		{
+			Name:   "json-number.bigint",
+			Input:  json.Number(strconv.FormatUint(math.MaxUint64, 10) + "999"),
+			Output: math.MaxUint64*1000.0 + 999, // Gets represented as a float64
+		},
+		{
+			Name:   "json-number.float",
+			Input:  json.Number("1234567890.42"),
+			Output: 1234567890.42,
+		},
+		{
+			Name:   "json-number.bogus",
+			Input:  json.Number("bogus"),
+			Output: "bogus",
+		},
 		{
 			Name: "struct",
 			Input: struct {
@@ -346,7 +368,7 @@ func TestEncodeDecode(t *testing.T) {
 				}
 
 				require.NoError(t, err, "unexpected error when decoding: %v", err)
-				require.True(t, reflect.DeepEqual(tc.Output, val), "expected %v, got %v", tc.Output, val)
+				require.True(t, reflect.DeepEqual(tc.Output, val), "expected %#v, got %#v", tc.Output, val)
 			})
 
 			t.Run("run", func(t *testing.T) {

internal/bindings/ctypes.go

@@ -137,14 +137,16 @@ func (w *WAFObject) SetMapKey(pinner pin.Pinner, key string) {
 	w.ParameterName = uintptr(unsafe.Pointer(header.Data))
 }
 
+var blankCStringValue = unsafe.Pointer(unsafe.NativeStringUnwrap("\x00").Data)
+
 // SetString sets the receiving [WAFObject] value to the given string.
 func (w *WAFObject) SetString(pinner pin.Pinner, str string) {
 	header := unsafe.NativeStringUnwrap(str)
 
 	w.Type = WAFStringType
 	w.NbEntries = uint64(header.Len)
 	if w.NbEntries == 0 {
-		w.Value = 0
+		w.Value = uintptr(blankCStringValue)
 		return
 	}
 	pinner.Pin(unsafe.Pointer(header.Data))

internal/bindings/waf_dl.go

@@ -65,12 +65,10 @@ func NewWAFLib() (dl *WAFLib, err error) {
 	}
 
 	if val := os.Getenv(log.EnvVarLogLevel); val != "" {
-		setLogSym, symErr := purego.Dlsym(handle, "ddwaf_set_log_cb")
-		if symErr != nil {
-			return nil, fmt.Errorf("get symbol: %w", symErr)
-		}
 		logLevel := log.LevelNamed(val)
-		dl.syscall(setLogSym, log.CallbackFunctionPointer(), uintptr(logLevel))
+		if logLevel != log.LevelOff {
+			dl.SetLogCb(log.CallbackFunctionPointer(), logLevel)
+		}
 	}
 
 	return

internal/log/log_cgo.go

@@ -8,7 +8,7 @@
 package log
 
 // #include "./ddwaf.h"
-// extern void ddwafLogCallbackFnv3(
+// extern void ddwafLogCallbackFnV4(
 //   DDWAF_LOG_LEVEL level,
 //   char* function,
 //   char* file,
@@ -22,11 +22,11 @@ import "github.com/DataDog/go-libddwaf/v4/internal/unsafe"
 // CallbackFunctionPointer returns a pointer to the log callback function which
 // can be used with libddwaf.
 func CallbackFunctionPointer() uintptr {
-	return uintptr(C.ddwafLogCallbackFnv3)
+	return uintptr(C.ddwafLogCallbackFnV4)
 }
 
-//export ddwafLogCallbackFnv3
-func ddwafLogCallbackFnv3(level C.DDWAF_LOG_LEVEL, fnPtr, filePtr *C.char, line C.unsigned, msgPtr *C.char, _ C.uint64_t) {
+//export ddwafLogCallbackFnV4
+func ddwafLogCallbackFnV4(level C.DDWAF_LOG_LEVEL, fnPtr, filePtr *C.char, line C.unsigned, msgPtr *C.char, _ C.uint64_t) {
 	function := unsafe.Gostring(unsafe.CastNative[C.char, byte](fnPtr))
 	file := unsafe.Gostring(unsafe.CastNative[C.char, byte](filePtr))
 	message := unsafe.Gostring(unsafe.CastNative[C.char, byte](msgPtr))

waf_test.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/DataDog/go-libddwaf/v4/internal/bindings"
 	"github.com/DataDog/go-libddwaf/v4/internal/lib"
-	"github.com/DataDog/go-libddwaf/v4/internal/log"
 	"github.com/DataDog/go-libddwaf/v4/timer"
 	"github.com/DataDog/go-libddwaf/v4/waferrors"
 	"github.com/stretchr/testify/require"
@@ -859,10 +858,6 @@ func TestConcurrency(t *testing.T) {
 		// This test validates that the WAF Context can be used from multiple
 		// threads, with mixed calls to `ddwaf_run` and `ddwaf_context_destroy`,
 		// which are not thread-safe.
-
-		wafLib.SetLogCb(log.CallbackFunctionPointer(), log.LevelDebug)
-		t.Cleanup(func() { wafLib.SetLogCb(0, log.LevelOff) })
-
 		waf, _, err := newDefaultHandle(testArachniRule)
 		require.NoError(t, err)