Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fuzzing] add cifuzz workflow #460

Merged
merged 4 commits into from
Sep 15, 2023
Merged

[Fuzzing] add cifuzz workflow #460

merged 4 commits into from
Sep 15, 2023

Conversation

0x34d
Copy link
Contributor

@0x34d 0x34d commented Aug 15, 2023

Add CIFuzz for PR fuzzing.

Signed-off-by: Arjun Singh <ajsinghyadav00@gmail.com>
@0x34d
Copy link
Contributor Author

0x34d commented Aug 15, 2023

panic: runtime error: slice bounds out of range [:2] with capacity 1 [recovered]
	panic: runtime error: slice bounds out of range [:2] with capacity 1
goroutine 17 [running, locked to thread]:
main.catchPanics()
	./main.2479209740.go:49 +0x35c
panic({0x8d70c0, 0x10c000296fd8})
	runtime/panic.go:884 +0x212
github.com/go-asn1-ber/asn1-ber.parseBinaryFloat({0x10c0002cba26, 0x2, 0x2})
	github.com/go-asn1-ber/asn1-ber@v1.5.4/real.go:98 +0x971
github.com/go-asn1-ber/asn1-ber.ParseReal({0x10c0002cba26, 0x2, 0x2})
	github.com/go-asn1-ber/asn1-ber@v1.5.4/real.go:45 +0x15e
github.com/go-asn1-ber/asn1-ber.readPacket({0x8e4628, 0x10c0002cfb00})
	github.com/go-asn1-ber/asn1-ber@v1.5.4/ber.go:382 +0xb17
github.com/go-asn1-ber/asn1-ber.readPacket({0x8e4628, 0x10c0002cfb00})
	github.com/go-asn1-ber/asn1-ber@v1.5.4/ber.go:318 +0x1a7a
github.com/go-asn1-ber/asn1-ber.DecodePacketErr({0x10c000296fc0, 0xb, 0x18})
	github.com/go-asn1-ber/asn1-ber@v1.5.4/ber.go:278 +0xaa
github.com/go-ldap/ldap.ParseDN({0x10c0002d8030, 0x25})
	github.com/go-ldap/ldap/dn.go:177 +0xb19
github.com/go-ldap/ldap.FuzzParseDN.func1(0x577f0c?, {0x10c0002d8030, 0x25})
	github.com/go-ldap/ldap/fuzz_test.go_fuzz.go:14 +0x66
reflect.Value.call({0x8ba700?, 0x8e38e0?, 0x5b8b94?}, {0x80fad1, 0x4}, {0x10c0002cfad0, 0x2, 0x2?})
	reflect/value.go:584 +0x1bce
reflect.Value.Call({0x8ba700?, 0x8e38e0?, 0x8101a3?}, {0x10c0002cfad0, 0x2, 0x2})
	reflect/value.go:368 +0x1fa
github.com/AdamKorcz/go-118-fuzz-build/testing.(*F).Fuzz(0x10c00026dd98, {0x8ba700?, 0x8e38e0})
	github.com/AdamKorcz/go-118-fuzz-build@v0.0.0-20230306123547-8075edf89bb0/testing/f.go:177 +0x9ab
github.com/go-ldap/ldap.FuzzParseDN(0x6a?)
	github.com/go-ldap/ldap/fuzz_test.go_fuzz.go:13 +0x65
main.LibFuzzerFuzzParseDN({0x60400015f390, 0x29, 0x29})
	./main.2479209740.go:31 +0x133
main.LLVMFuzzerTestOneInput(0x60400015f390, 0x29)
	./main.2479209740.go:24 +0xa5
AddressSanitizer:DEADLYSIGNAL
=================================================================
==72==ERROR: AddressSanitizer: ABRT on unknown address 0x000000000048 (pc 0x0000005d2561 bp 0x10c00026ca08 sp 0x10c00026c9f0 T0)
SCARINESS: 10 (signal)
    #0 0x5d2561 in runtime.raise.abi0 runtime/sys_linux_amd64.s:159
DEDUP_TOKEN: runtime.raise.abi0
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT runtime/sys_linux_amd64.s:159 in runtime.raise.abi0
==72==ABORTING
MS: 3 ChangeByte-ShuffleBytes-ShuffleBytes-; base unit: ba1a702c6e9d417c466860048e532edd65d507dc
0x25,0x0,0x0,0x0,0x32,0x0,0x66,0x0,0x30,0x0,0x65,0xff,0x10,0x69,0x0,0x29,0xfe,0x3d,0x23,0x32,0x32,0x32,0x32,0x32,0x30,0x30,0x30,0x30,0x39,0x30,0x32,0x39,0x35,0x38,0x35,0x34,0x34,0x30,0x31,0x36,0x66,
%\000\000\0002\000f\0000\000e\377\020i\000)\376=#222220000902958544016f
artifact_prefix='/tmp/tmp8w4aewlw/'; Test unit written to /tmp/tmp8w4aewlw/crash-b378e831f3c02c9063d68849a35e10b6022ff041
Base64: JQAAADIAZgAwAGX/EGkAKf49IzIyMjIyMDAwMDkwMjk1ODU0NDAxNmY=

Crash happen due to #452 .

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

@cpuschma What's going on? Is there some bug or something? Do I need to open a new PR?

Or maybe you don't have rights to merge on workflow.

@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

update: there is a out-of-memory bug in fuzz_parse_dn

Loaded 1024/1265 files from /github/workspace/cifuzz-corpus/fuzz_parse_dn
==45== ERROR: libFuzzer: out-of-memory (used: 3537Mb; limit: 2560Mb)
   To change the out-of-memory limit use -rss_limit_mb=<N>

Live Heap Allocations: 25579636 bytes in 6281 chunks; quarantined: 9270093 bytes in 5812 chunks; 26266 other chunks; total chunks: 38359; showing top 95% (at most 8 unique contexts)
24120920 byte(s) (94%) in 11 allocation(s)
    #0 0x52efb6 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
    #1 0x4ad9b7 in operator new(unsigned long) cxa_noexception.cpp
    #2 0x458362 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #3 0x7f7601eb6082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)

DEDUP_TOKEN: __interceptor_malloc--operator new(unsigned long)--main
473804 byte(s) (1%) in 1253 allocation(s)
    #0 0x52efb6 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
    #1 0x4ad9b7 in operator new(unsigned long) cxa_noexception.cpp
    #2 0x43d377 in fuzzer::Fuzzer::RereadOutputCorpus(unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:418:3
    #3 0x43f8ed in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:873:7
    #4 0x42ed0f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6
    #5 0x458362 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #6 0x7f7601eb6082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)

Job: bug
But I'm unable to reproduce.

@0x34d 0x34d closed this Sep 15, 2023
@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

Trigger CI

@0x34d 0x34d reopened this Sep 15, 2023
@cpuschma
Copy link
Member

cpuschma commented Sep 15, 2023

I am not familiar with Google's fuzzing library, but I would interpret the message as an indication that with enough weird input ParseDN takes up a lot of memory. I triggered the CI to run again and the OOM happened again.

Correct me if I'm wrong here, this is not great, but no reason to hold back the PR and not merge first?

@TomSellers
Copy link
Contributor

TomSellers commented Sep 15, 2023

RE: the memory spike - I just started up some local fuzzing of the same fuzzer using the following command:
go test -v -fuzz ^FuzzParseDN$ github.com/go-ldap/ldap

I definitely see some large memory usage (1.9 GB) happening. I haven't looked into why yet. In my experience this is often a call to make() with a size that is massive. I have not figured out a way to check memory allocation of a specific fuzz corpus entry when running fuzzing so I've just had to dig through the code.

@cpuschma
Copy link
Member

I think it's best to address this in a separate PR afterwards. Let's get this merged first.

@cpuschma cpuschma merged commit 16bdf0b into go-ldap:master Sep 15, 2023
20 checks passed
@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

I am not familiar with Google's fuzzing library, but I would interpret the message as an indication that with enough weird input ParseDN takes up a lot of memory. I triggered the CI to run again and the OOM happened again.

Correct me if I'm wrong here, this is not great

Yes, malformed input is taking a lot of memory.

But in my local run:

python infra/helper.py build_image  go-ldap
python infra/helper.py build_fuzzers go-ldap --engine libfuzzer --sanitizer address
python infra/helper.py run_fuzzer go-ldap fuzz_parse_dn

Everything looks fine.

@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

Update: If this OOM is a real bug in oss-fuzz system ID: 61512

@TomSellers
Copy link
Contributor

TomSellers commented Sep 15, 2023

Pretty sure I know of a case that can spike the memory because I had to fix it elsewhere.

ParseDN calls ber.DecodePacketErr

ldap/dn.go

Line 177 in 03cc78c

packet, err := ber.DecodePacketErr(rawBER)

ber.DecodePacketErr calls readPacket
https://github.com/go-asn1-ber/asn1-ber/blob/04301b4b1c5ff66221f8f8a394f814a9917d678a/ber.go#L285-L286

readPacket calls make([]byte, length) where length is attacker controlled and, due to limits, has to be less than MaxPacketLengthBytes which is math.MaxInt32 .
https://github.com/go-asn1-ber/asn1-ber/blob/04301b4b1c5ff66221f8f8a394f814a9917d678a/ber.go#L352-L355

This means a malicious/malformed packet can cause a 2 GB []byte to be allocated briefly. This is a problem when fuzzing because you often have multiple parallel threads allocating up to 2 GB each. In my tests I was blowing through 32 GB easily.

You can fix this by setting ber.MaxPacketLengthBytes to some reasonable value in the calling code. For example:
ber.MaxPacketLengthBytes = 65536

This worked well in my fuzz testing.

@0x34d
Copy link
Contributor Author

0x34d commented Sep 15, 2023

You can fix this by setting ber.MaxPacketLengthBytes to some reasonable value in the calling code. For example:
ber.MaxPacketLengthBytes = 65536

Mega lol, this is clever!
It's a garbage-in, garbage-out (GIGO) bug. The best fix is to set the maximum input size to the max UDP packet size.

@cpuschma
Copy link
Member

Just noticed you added a limit in the go-asn1-ber libraries fuzzing functions:

func FuzzDecodePacket(f *testing.F) {
	// Set a limit on the length decoded in readPacket() since the call to
	// make([]byte, length) can allocate up to MaxPacketLengthBytes which is
	// currently 2 GB. This can cause memory related crashes when fuzzing in
	// parallel or on memory constrained devices.
	MaxPacketLengthBytes = 65536

I'd suggest to do the same in our fuzzing. It's unfortunate that this is a package-wide setting in the ASN1 BER library. We should look into some sort of encoder/decoder structs like JSON.NewDecoder with configurable settings instead in the future, to not interfere with other uses of the library.

@TomSellers
Copy link
Contributor

TomSellers commented Sep 15, 2023

Ah, yeah, I forgot I upstreamed that.

It's unfortunate that this is a package-wide setting in the ASN1 BER library.

At least MaxPacketLengthByte is only used by readPacket (and everything that calls it) so it should have some controlled impact. I set it to 65536 because it was large enough to accommodate most large LDAP responses from things like Active Directory and it is sufficiently large to let the fuzzer exercise the code. That said, I haven't battle tested it vs AD with a ton of users/groups.

@cpuschma
Copy link
Member

cpuschma commented Sep 15, 2023

Depending on the size of the tree, 65KB might not be sufficient. I know atleast of three cases where the directory tree is huge. With that said, the response is split across multiple responses in most common DS. Unfortunately, this doesn't account for malicious servers.

In addition, we can't just lower the maximum packet size as this would be a non-calculatable risk to break things. I think it would be for the best to address this in the ASN1 BER library first by creating encoders/decoders and then implement them in this library.

cpuschma added a commit that referenced this pull request Sep 15, 2023
…466)

Parallel and large amount of fuzzing data can create large amounts of allocated data and cause restricted fuzzing environments to crash (see #460)
gustavoluvizotto pushed a commit to gustavoluvizotto/ldap-fork that referenced this pull request Apr 11, 2024
Signed-off-by: Arjun Singh <ajsinghyadav00@gmail.com>
Co-authored-by: Christopher Puschmann <cp@lumen.sh>
gustavoluvizotto pushed a commit to gustavoluvizotto/ldap-fork that referenced this pull request Apr 11, 2024
…o-ldap#466)

Parallel and large amount of fuzzing data can create large amounts of allocated data and cause restricted fuzzing environments to crash (see go-ldap#460)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants