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

Release v0.27.4 #4756

Merged
merged 4 commits into from Aug 6, 2018
Merged

Release v0.27.4 #4756

merged 4 commits into from Aug 6, 2018

Conversation

pks-t
Copy link
Member

@pks-t pks-t commented Aug 6, 2018

This is a security release fixing out-of-bounds reads when
processing smart-protocol "ng" packets.

When parsing an "ng" packet, we keep track of both the current position
as well as the remaining length of the packet itself. But instead of
taking care not to exceed the length, we pass the current pointer's
position to strchr, which will search for a certain character until
hitting NUL. It is thus possible to create a crafted packet which
doesn't contain a NUL byte to trigger an out-of-bounds read.

The issue was discovered by the oss-fuzz project, issue 9406.

@pks-t pks-t force-pushed the pks/v0.27.4 branch 2 times, most recently from 1b9b30c to 7b06172 Compare August 6, 2018 06:41
Travis has upgraded the default Xcode images from 8.3 to 9.4 on 31st
July 2018, including an upgrade to macOS 10.13. Unfortunately, this
breaks our CI builds on our maintenance branches. As we do not want to
include mayor changes to fix the integration right now, we force use of
the old Xcode 8.3 images.
OSS-fuzz has reported a potential out-of-bounds read when processing a
"ng" smart packet:

==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6310000249c0 at pc 0x000000493a92 bp 0x7ffddc882cd0 sp 0x7ffddc882480
	READ of size 65529 at 0x6310000249c0 thread T0
	SCARINESS: 26 (multi-byte-read-heap-buffer-overflow)
	#0 0x493a91 in __interceptor_strchr.part.35 /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:673
	#1 0x813960 in ng_pkt libgit2/src/transports/smart_pkt.c:320:14
	#2 0x810f79 in git_pkt_parse_line libgit2/src/transports/smart_pkt.c:478:9
	#3 0x82c3c9 in git_smart__store_refs libgit2/src/transports/smart_protocol.c:47:12
	#4 0x6373a2 in git_smart__connect libgit2/src/transports/smart.c:251:15
	libgit2#5 0x57688f in git_remote_connect libgit2/src/remote.c:708:15
	libgit2#6 0x52e59b in LLVMFuzzerTestOneInput /src/download_refs_fuzzer.cc:145:9
	libgit2#7 0x52ef3f in ExecuteFilesOnyByOne(int, char**) /src/libfuzzer/afl/afl_driver.cpp:301:5
	libgit2#8 0x52f4ee in main /src/libfuzzer/afl/afl_driver.cpp:339:12
	libgit2#9 0x7f6c910db82f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291
	libgit2#10 0x41d518 in _start

When parsing an "ng" packet, we keep track of both the current position
as well as the remaining length of the packet itself. But instead of
taking care not to exceed the length, we pass the current pointer's
position to `strchr`, which will search for a certain character until
hitting NUL. It is thus possible to create a crafted packet which
doesn't contain a NUL byte to trigger an out-of-bounds read.

Fix the issue by instead using `memchr`, passing the remaining length as
restriction. Furthermore, verify that we actually have enough bytes left
to produce a match at all.

OSS-Fuzz-Issue: 9406
@pks-t pks-t merged commit 8b89f36 into libgit2:maint/v0.27 Aug 6, 2018
@pks-t pks-t deleted the pks/v0.27.4 branch August 6, 2018 08:49
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.

None yet

1 participant