memory hogging

[0x34d]

Describe the bug

ReadFrame parser is using a lot of RAM.

Reproduction steps

1. [Fuzzing] Update the fuzzing suite to native Go-Fuzz #212
2. make fuzzing
3. System crash

[lukebakken]

Hello, we would really appreciate it if you would explain the new test a bit more, and hopefully even provide a fix for it.

[0x34d]

Hello, we would really appreciate it if you would explain the new test a bit more, and hopefully even provide a fix for it.

I did some investigation, but it was disappointing as I couldn't pinpoint the root cause. My apologies, my expertise lies with C, not Go.

In my view, a basic parser shouldn't exhaust all the RAM simply for parsing string data.

If this were a server, I could have shown you a magic trick, but it's a client.

[lukebakken]

In my view, a basic parser shouldn't exhaust all the RAM simply for parsing string data

Sure, but how would invalid data make it to the parser in a real-world client application?

[0x34d]

In my view, a basic parser shouldn't exhaust all the RAM simply for parsing string data

Sure, but how would invalid data make it to the parser in a real-world client application?

While it might not be a real-world problem, it's an issue for me as the high memory usage prevents me from conducting proper fuzz testing.

test code :

func TestMemoryHog(t *testing.T) {
	input_data := []byte("\x01000000\x00(\x00\x1e\x00\x00\x00\x00\x04\x00\x0400\xf3q\x19\x19\x19\x19")

	for i := 0; i < 5000; i++ {
		r := reader{bytes.NewReader(input_data)}
		_, _ = r.ReadFrame()
	}
}

run : go test -run=TestMemoryHog

Problem: During the execution of this test, the memory usage was approximately 1.7GB, I don't believe this is normal.

[lukebakken]

Problem: During the execution of this test, the memory usage was approximately 1.7GB, I don't believe this is normal.

Actually, it is expected. Your input data eventually ends up requesting 1897470233 bytes here:

https://github.com/rabbitmq/amqp091-go/blob/main/read.go#L117

There is no maximum message size specified by the AMQP 0.9.1 protocol, so your input will cause this library to request that much memory. Even though a subsequent operation returns an error, the Go memory manager won't release this memory that it has just requested from the operating system, hoping to use it again.

Your input_data is a great example of garbage in, garbage out.

@Zerpet @Gsantomaggio what do you think of enacting a maximum frame and message size in this library? The .NET client has something like it:

• Add ability to specify maximum message size rabbitmq-dotnet-client#1220
• MaxMessageSize actually limits frame size rabbitmq-dotnet-client#1356

[lukebakken]

#215

If anything the new fuzz test shows that this library can be made to allocate huge amounts of memory in the case of malformed input data. I tried limiting one allocation to 65KiB and it did help a bit with the fuzz test, but some workers' memory use was still high.

This is an interesting problem and may be one we'd like to look into when we have time, but it will be low priority.

cc @Zerpet @Gsantomaggio

[lukebakken]

it's an issue for me as the high memory usage prevents me from conducting proper fuzz testing

Just out of curiosity, why are you doing this testing on this library? Research?

[0x34d]

Yes. I like network-related stuff.