OSS-Fuzz issues

[xhanulik]

Recently reported issues by OSS-Fuzz.

Checklist

• PKCS#11 module is tested
• Windows minidriver is tested
• macOS tokend is tested

[Jakuje]

I do not see how the first commit fixes the referenced issue. Can you clarify? It just improves the checking of the SW (which is certainly good), but if the fuzzer will come with the SW 90 00, it will fail again, isn't it? This sounds like some issue that the pubkey->u.rsa.modulus.data is not allocated in either of the code paths.

For the second commit, could you add also a reproducer unit tests with some similar input to the fuzzer one to better understand the issue?

[xhanulik]

Sure, the problem is NULL dereference when calling SHA1 on pubkey->u.rsa.modulus.data, which should be allocated in starcos_gen_key. During the communication with fuzzed card, it returns 90 20, what is in the if statement

OpenSC/src/libopensc/card-starcos.c

Lines 1535 to 1536 in df58bfc

	if (apdu.sw1 != 0x90 || apdu.sw2 != 0x00)
	return sc_check_sw(card, apdu.sw1, apdu.sw2);

considered as error, but sc_check_sw does not return error, so the pubkey->u.rsa.modulus.data is not allocated, but the error is not propagated.

Thanks for pointing out, it should be clarified in the commit message now. The reproducer for the second one is also added to unit tests.

[Jakuje]

I would prefer to get the formatting right, but otherwise lgtm

[frankmorgner]

The fuzzer reports a memory leak, which seems to be unrelated, but should be fixed at some point anyway:

Direct leak of 160 byte(s) in 1 object(s) allocated from:
    #0 0x5627fe57931e in malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
    #1 0x5627fe6477ba in sc_pkcs15_read_cached_file /src/opensc/src/libopensc/pkcs15-cache.c:158:10
    #2 0x5627fe5fde87 in sc_pkcs15_read_file /src/opensc/src/libopensc/pkcs15.c:2488:7
    #3 0x5627fe610644 in sc_pkcs15_read_certificate /src/opensc/src/libopensc/pkcs15-cert.c:387:7
    #4 0x5627fe9fba34 in sc_pkcs15emu_cac_init /src/opensc/src/libopensc/pkcs15-cac.c:277:8
    #5 0x5627fe9f8329 in sc_pkcs15emu_cac_init_ex /src/opensc/src/libopensc/pkcs15-cac.c:380:9
    #6 0x5627fe649d0f in sc_pkcs15_bind_synthetic /src/opensc/src/libopensc/pkcs15-syn.c:139:8
    #7 0x5627fe5f5d35 in sc_pkcs15_bind /src/opensc/src/libopensc/pkcs15.c:1359:8
    #8 0x5627fe5b6955 in LLVMFuzzerTestOneInput /src/opensc/src/tests/fuzzing/fuzz_pkcs15_decode.c:78:2
    #9 0x5627fe46ad80 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
    #10 0x5627fe46a5a5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:516:7
    #11 0x5627fe46bd75 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:760:19
    #12 0x5627fe46cb65 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile>>&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:905:5
    #13 0x5627fe45ae76 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:914:6
    #14 0x5627fe4873a2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #15 0x7fd2e47ba082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 87b331c034a6458c64ce09c03939e947212e18ce)

[xhanulik]

The fuzzer reports a memory leak, which seems to be unrelated, but should be fixed at some point anyway

I cannot reproduce it or either find a place, where the reported leak is happening.

[Jakuje]

The fuzzer reports a memory leak, which seems to be unrelated, but should be fixed at some point anyway

I cannot reproduce it or either find a place, where the reported leak is happening.

Would not it be something like this?

diff --git a/src/libopensc/pkcs15.c b/src/libopensc/pkcs15.c
index bb03ac7ec..3f3198d3b 100644
--- a/src/libopensc/pkcs15.c
+++ b/src/libopensc/pkcs15.c
@@ -2549,6 +2549,7 @@ sc_pkcs15_read_file(struct sc_pkcs15_card *p15card, const struct sc_path *in_pat
 			}
 		}
 
+		free(data);
 		data = malloc(len);
 		if (data == NULL) {
 			r = SC_ERROR_OUT_OF_MEMORY;

If I see right, the memory is allocated in sc_pkcs15_read_cached_file() but not freed before this allocation happens in case the file cache is read, but the file selection fails.

[xhanulik]

If I see right, the memory is allocated in sc_pkcs15_read_cached_file() but not freed before this allocation happens in case the file cache is read, but the file selection fails.

Thanks, I missed the !r before file selection.

[frankmorgner]

thank you