Conversation
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
asn.c:1514: cert->subjectCN = (char *)&cert->source[cert->srcIdx];. Same to assume pointer to cert->source will stay around?
asn.c:1960: XMEMCPY(&cert->publicKey, &cert->source[tmpIdx], pubLen);. Maybe check pubLen does not exceed what cert->publicKey can hold?
asn.c:2053: MakeSigner. Is that code for AddCA really needed for this effort? Signer appears to be a dynamic list that goes into Certificate Manager cm->caTable. Also appears the AddCA was moved from ssl.c?
wolfssl/wolfcrypt/asn.h
Outdated
| #ifndef WOLFSSL_NO_MALLOC | ||
| byte* digest; | ||
| #else | ||
| byte digest[512]; |
wolfcrypt/src/random.c
Outdated
| #ifndef WOLFSSL_NO_MALLOC | ||
| XFREE(rng, heap, DYNAMIC_TYPE_RNG); | ||
| #else | ||
| for (int i = 0; i < WOLFSSL_DO178_MAX_RNG; i++) { |
There was a problem hiding this comment.
Recommend avoiding int i declaration here. Put at top of function or brace.
wolfcrypt/src/ecc.c
Outdated
| #ifndef FP_LUT | ||
| #define FP_LUT 8U | ||
| WOLFSSL_ABI | ||
| ecc_key* wc_ecc_key_new(void* heap) |
There was a problem hiding this comment.
Does the customer really need these wc_ecc_key_new() and wc_ecc_key_free() API's?
|
Thanks for the review @dgarske. You are correct that AddCA(), wc_ecc_key_new() and wc_ecc_key_free() are probably not needed. I must have overthought about the need to parse and verify multiple CAs. Good point on asn.c:1514: cert->subjectCN = (char *)&cert->source[cert->srcIdx]; |
otherwise if profile_str_len is > strlen(gSrtpProfiles[i].name) we end up
comparing memory past gSrtpProfiles[i].name. -fsanitize=address catches this:
```
==100159==ERROR: AddressSanitizer: global-buffer-overflow on address 0x7f40d8d533b2 at pc 0x7f40d8eb014f bp 0x7f40d50fe240 sp 0x7f40d50fd9e8
READ of size 21 at 0x7f40d8d533b2 thread T107
#0 0x7f40d8eb014e in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long) /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:860
#1 0x7f40d8eb06e6 in __interceptor_memcmp /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:892
#2 0x7f40d8eb06e6 in __interceptor_memcmp /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:887
#3 0x7f40d8c2e830 in DtlsSrtpFindProfile src/ssl.c:1310
#4 0x7f40d8c2e9ed in DtlsSrtpSelProfiles src/ssl.c:1347
#5 0x7f40d8c2eada in wolfSSL_CTX_set_tlsext_use_srtp src/ssl.c:1359
#6 0x563bf381b4c5 in server_test examples/server/server.c:2278
wolfSSL#7 0x7f40d88f0258 in start_thread (/usr/lib/libpthread.so.0+0x9258)
wolfSSL#8 0x7f40d88195e2 in __GI___clone (/usr/lib/libc.so.6+0xfe5e2)
```
2 participants
For review