Skip to content

use-after-free in GenerateHashString #615

Description

@dnovikoff

datamodel/low/extraction_functions.go#L1137

The problem is as follows.
Function takes "any" to calculate its hash.

func GenerateHashString(v any) string {

The reflect value

val := reflect.ValueOf(v)

and a Pointer

cacheKey := val.Pointer()

which is unitptr (does not hold object)

If the last pointer to the object (v) is lost, the memory is freed by GC.
Now the new object may take the same adress.
And when the GenerateHashString is called for that new object, the hash(string) is not calculated again. It is taken from the cache. And this value is invalid.

Can be reproduced with the test

func TestGenerateHashString_PointerCacheUseAfterFree(t *testing.T) {
	mk := func(value string) *yaml.Node {
		return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: value}
	}

	// 1. Hash nodeA (content A) → cache[nodeA_ptr] = hashA.
	nodeA := mk("content-A")
	hashA := GenerateHashString(nodeA)
	aPtr := reflect.ValueOf(nodeA).Pointer()

	// 2. Drop nodeA and reclaim it. uintptr key does not keep it alive.
	nodeA = nil
	runtime.GC()

	// 3. Allocate until a node lands at nodeA's old address (the swept slot is
	//    returned by the allocator within a few same-size allocations).
	var nodeB *yaml.Node
	for i := 0; i < 1024; i++ {
		nodeB = mk("content-B") // content B must hash != A
		if reflect.ValueOf(nodeB).Pointer() == aPtr {
			break
		}
	}
	if reflect.ValueOf(nodeB).Pointer() != aPtr {
		t.Skipf("address not reused within bound; cannot demonstrate use-after-free (aPtr=%x)", aPtr)
	}

	// 4. nodeB now lives at nodeA's old address; the cache still holds hashA.
	got := GenerateHashString(nodeB)

	// 5. Correct hash for nodeB, recomputed with a clean cache.
	hashCache.Delete(aPtr)
	want := GenerateHashString(nodeB)

	if hashA == want {
		t.Fatalf("test setup error: contents A and B hash the same; cannot demonstrate staleness")
	}
	if got != want {
		t.Fatalf("use-after-free: GenerateHashString(nodeB) returned the stale hash of freed nodeA\n got(stale)=%s\n want(fresh)=%s", got, want)
	}
}

The fast fix is to change

shouldCache := true

into

shouldCache := false

which disables global caching.

We found this issue while trying to free(nil) loaded Document after parsing to our internal model to avoid extra memory usage.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions