fix: data race between libddwaf.Load() and libddwaf.Usable() (#149)

A data race existed between `libddwaf.Load()` and `libddwaf.Usable()` as
both access shared global state that is not managed through atomic
pointers.

This PR adds a `sync.Mutex` to resolve the race condition between `Load`
and `Usable`; as the use of a `sync.Once` in `Load` ensures there is no
race condition in other places (all uses of the `gWafLib` are guaranteed
to be preceded by a call to `Load`, which means the reads "synchronize
after" the write).

Author: RomainMuller

alignement_test.go

@@ -29,7 +29,7 @@ func TestWafObject(t *testing.T) {
 	_, err := Load()
 	require.NoError(t, err)
 
-	lib := wafLib.Handle()
+	lib := gWafLib.Handle()
 
 	t.Run("invalid", func(t *testing.T) {
 		var actual bindings.WAFObject

builder.go

@@ -37,7 +37,7 @@ func NewBuilder(keyObfuscatorRegex string, valueObfuscatorRegex string) (*Builde
 
 	var pinner runtime.Pinner
 	defer pinner.Unpin()
-	hdl := wafLib.BuilderInit(newConfig(&pinner, keyObfuscatorRegex, valueObfuscatorRegex))
+	hdl := gWafLib.BuilderInit(newConfig(&pinner, keyObfuscatorRegex, valueObfuscatorRegex))
 
 	if hdl == 0 {
 		return nil, errors.New("failed to initialize the WAF builder")
@@ -51,7 +51,7 @@ func (b *Builder) Close() {
 	if b == nil || b.handle == 0 {
 		return
 	}
-	wafLib.BuilderDestroy(b.handle)
+	gWafLib.BuilderDestroy(b.handle)
 	b.handle = 0
 }
 
@@ -122,9 +122,9 @@ func (b *Builder) AddOrUpdateConfig(path string, fragment any) (Diagnostics, err
 // Returns the [Diagnostics] produced by adding or updating this configuration.
 func (b *Builder) addOrUpdateConfig(path string, cfg *bindings.WAFObject) (Diagnostics, error) {
 	var diagnosticsWafObj bindings.WAFObject
-	defer wafLib.ObjectFree(&diagnosticsWafObj)
+	defer gWafLib.ObjectFree(&diagnosticsWafObj)
 
-	res := wafLib.BuilderAddOrUpdateConfig(b.handle, path, cfg, &diagnosticsWafObj)
+	res := gWafLib.BuilderAddOrUpdateConfig(b.handle, path, cfg, &diagnosticsWafObj)
 
 	var diags Diagnostics
 	if !diagnosticsWafObj.IsInvalid() {
@@ -150,7 +150,7 @@ func (b *Builder) RemoveConfig(path string) bool {
 		return false
 	}
 
-	return wafLib.BuilderRemoveConfig(b.handle, path)
+	return gWafLib.BuilderRemoveConfig(b.handle, path)
 }
 
 // ConfigPaths returns the list of currently loaded configuration paths.
@@ -159,7 +159,7 @@ func (b *Builder) ConfigPaths(filter string) []string {
 		return nil
 	}
 
-	return wafLib.BuilderGetConfigPaths(b.handle, filter)
+	return gWafLib.BuilderGetConfigPaths(b.handle, filter)
 }
 
 // Build creates a new [Handle] instance that uses the current configuration.
@@ -171,7 +171,7 @@ func (b *Builder) Build() *Handle {
 		return nil
 	}
 
-	hdl := wafLib.BuilderBuildInstance(b.handle)
+	hdl := gWafLib.BuilderBuildInstance(b.handle)
 	if hdl == 0 {
 		return nil
 	}

context.go

@@ -225,12 +225,12 @@ func (context *Context) run(persistentData, ephemeralData *bindings.WAFObject, r
 
 	var result bindings.WAFObject
 	pinner.Pin(&result)
-	defer wafLib.ObjectFree(&result)
+	defer gWafLib.ObjectFree(&result)
 
 	// The value of the timeout cannot exceed 2^55
 	// cf. https://en.cppreference.com/w/cpp/chrono/duration
 	timeout := uint64(runTimer.SumRemaining().Microseconds()) & 0x008FFFFFFFFFFFFF
-	ret := wafLib.Run(context.cContext, persistentData, ephemeralData, &result, timeout)
+	ret := gWafLib.Run(context.cContext, persistentData, ephemeralData, &result, timeout)
 
 	decodeTimer := runTimer.MustLeaf(DecodeTimeKey)
 	decodeTimer.Start()
@@ -324,7 +324,7 @@ func (context *Context) Close() {
 	context.mutex.Lock()
 	defer context.mutex.Unlock()
 
-	wafLib.ContextDestroy(context.cContext)
+	gWafLib.ContextDestroy(context.cContext)
 	defer context.handle.Close() // Reduce the reference counter of the Handle.
 	context.cContext = 0         // Makes it easy to spot use-after-free/double-free issues
 

handle.go

@@ -55,7 +55,7 @@ func (handle *Handle) NewContext(timerOptions ...timer.Option) (*Context, error)
 		return nil, fmt.Errorf("handle was released")
 	}
 
-	cContext := wafLib.ContextInit(handle.cHandle)
+	cContext := gWafLib.ContextInit(handle.cHandle)
 	if cContext == 0 {
 		handle.Close() // We couldn't get a context, so we no longer have an implicit reference to the Handle in it...
 		return nil, fmt.Errorf("could not get C context")
@@ -77,13 +77,13 @@ func (handle *Handle) NewContext(timerOptions ...timer.Option) (*Context, error)
 // Addresses returns the list of addresses the WAF has been configured to monitor based on the input
 // ruleset.
 func (handle *Handle) Addresses() []string {
-	return wafLib.KnownAddresses(handle.cHandle)
+	return gWafLib.KnownAddresses(handle.cHandle)
 }
 
 // Actions returns the list of actions the WAF has been configured to monitor based on the input
 // ruleset.
 func (handle *Handle) Actions() []string {
-	return wafLib.KnownActions(handle.cHandle)
+	return gWafLib.KnownActions(handle.cHandle)
 }
 
 // Close decrements the reference counter of this [Handle], possibly allowing it to be destroyed
@@ -95,7 +95,7 @@ func (handle *Handle) Close() {
 		return
 	}
 
-	wafLib.Destroy(handle.cHandle)
+	gWafLib.Destroy(handle.cHandle)
 	handle.cHandle = 0 // Makes it easy to spot use-after-free/double-free issues
 }
 

waf.go

@@ -16,10 +16,15 @@ import (
 // Globally dlopen() libddwaf only once because several dlopens (eg. in tests)
 // aren't supported by macOS.
 var (
-	// libddwaf's dynamic library handle and entrypoints
-	wafLib *bindings.WAFLib
-	// libddwaf's dlopen error if any
-	wafLoadErr  error
+	// libddwaf's dynamic library handle and entrypoints. This is only safe to
+	// read after calling [Load] or having acquired [gMu].
+	gWafLib *bindings.WAFLib
+	// libddwaf's dlopen error if any. This is only safe to read after calling
+	// [Load] or having acquired [gMu].
+	gWafLoadErr error
+	// Protects the global variables above.
+	gMu sync.Mutex
+
 	openWafOnce sync.Once
 )
 
@@ -41,14 +46,19 @@ func Load() (bool, error) {
 	}
 
 	openWafOnce.Do(func() {
-		wafLib, wafLoadErr = bindings.NewWAFLib()
-		if wafLoadErr != nil {
+		// Acquire the global state mutex so we don't have a race condition with
+		// [Usable] here.
+		gMu.Lock()
+		defer gMu.Unlock()
+
+		gWafLib, gWafLoadErr = bindings.NewWAFLib()
+		if gWafLoadErr != nil {
 			return
 		}
-		wafVersion = wafLib.GetVersion()
+		wafVersion = gWafLib.GetVersion()
 	})
 
-	return wafLib != nil, wafLoadErr
+	return gWafLib != nil, gWafLoadErr
 }
 
 var wafVersion string
@@ -76,5 +86,9 @@ func Usable() (bool, error) {
 	wafSupportErrors := errors.Join(support.WafSupportErrors()...)
 	wafManuallyDisabledErr := support.WafManuallyDisabledError()
 
-	return (wafLib != nil || wafLoadErr == nil) && wafSupportErrors == nil && wafManuallyDisabledErr == nil, errors.Join(wafLoadErr, wafSupportErrors, wafManuallyDisabledErr)
+	// Acquire the global state mutex as we are not calling [Load] here, so we
+	// need to explicitly avoid a race condition with it.
+	gMu.Lock()
+	defer gMu.Unlock()
+	return (gWafLib != nil || gWafLoadErr == nil) && wafSupportErrors == nil && wafManuallyDisabledErr == nil, errors.Join(gWafLoadErr, wafSupportErrors, wafManuallyDisabledErr)
 }

waf_test.go

@@ -358,6 +358,9 @@ func TestTimeout(t *testing.T) {
 	})
 
 	t.Run("both-timeout", func(t *testing.T) {
+		// TODO(eliott.bouhana): APPSEC-58637
+		t.Skip("TODO(eliott.bouhana): This test is flaky and needs to be fixed")
+
 		context, err := waf.NewContext(timer.WithBudget(time.Millisecond), timer.WithComponents(wafTimerKey, raspTimerKey))
 		require.NoError(t, err)
 		require.NotNil(t, context)