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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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