On Thu, Oct 08, 2015 at 01:36:07PM +0000, Pascal Cuoq via RT wrote: > > - ssl_locl.h.patch: I don't see a struct timeval > > crypto/x509v3/v3_scts.c. Does this comment still apply? Maybe > > we fixed the issue in some other way. > > Sorry, this comment was unnecessarily confusing. > > What we meant to say was that the OpenSSL header ssl/ssl_locl.h uses "struct > timeval" (at line 1445: > https://github.com/openssl/openssl/blob/b3e2272c59a5720467045e2ae62940fdb708ce76/ssl/ssl_locl.h#L1445 > ) but the POSIX header that is guaranteed to provide this type, sys/time.h, > is not included. This is just a portability matter. It works in practice of > most platforms because that header is dragged in by other headers.
I understand that it's a portability issue. But since it's not part of the C standard we can't unconditionally include it either because that would create other protability issues. d1_lib.c for instance has: #if defined(OPENSSL_SYS_VMS) # include <sys/timeb.h> #elif defined(OPENSSL_SYS_NETWARE) && !defined(_WINSOCK2API_) # include <sys/timeval.h> #elif defined(OPENSSL_SYS_VXWORKS) # include <sys/times.h> #elif !defined(OPENSSL_SYS_WIN32) # include <sys/time.h> #endif While bss_dgram.c just has: # if !(defined(_WIN32) || defined(OPENSSL_SYS_VMS)) # include <sys/time.h> # endif # if defined(OPENSSL_SYS_VMS) # include <sys/timeb.h> # endif > > - malloc.patch: I started looking at it, and I have some > > comments/questions: > > - I currently don't see a way that s->d1 can be NULL except > > after an dtls1_free() call. The same seem to go for > > DTLS_RECORD_LAYER_free(), ssl3_free(), pkey_hmac_cleanup(), > > aes_gcm_cleanup() and aes_ocb_cleanup(). > > Are you saying there are cases we could end up calling those > > twice? > > All the patches in the file malloc.patch are for problems, typically null > pointer dereferences, that arise when the function malloc() is modeled as > either returning a null pointer or a valid pointer to a fresh block. We know > this because we also ran all the tests in a mode in which malloc was assumed > to always succeed, and in that mode, none of the fixes recommended in > malloc.patch were necessary. > > So for instance, in the development version of OpenSSL that was current at > the time of the report, that "s->d1" could be NULL because a malloc call had > returned NULL. I see a check for the malloc() return value there, and as far as I can see it's always been there. But you only mention the free calls while the variables are used at other places without checking that it's NULL or not. So I wonder if this is some false positive, which I understand you actually try to avoid. So I'm guessing we're missing something, like calling free after new failed or something. > - replay the same verification process on the most recent version of OpenSSL, > and update this bug report with the issues that are still present only; > > - and then give you access to the analyzer results in a way that lets you > observe the sequence of events leading to the null pointer dereferences, and > choose for yourselves the best remedy. That would be great. Kurt _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
