mirror of
https://github.com/Mbed-TLS/mbedtls.git
synced 2025-01-27 15:35:50 +00:00
a3d831b9e6
This uncovered a bug that led to a double-free (in practice, in general could be free() on any invalid value): initially the session structure is loaded with `memcpy()` which copies the previous values of pointers peer_cert and ticket to heap-allocated buffers (or any other value if the input is attacker-controlled). Now if we exit before we got a chance to replace those invalid values with valid ones (for example because the input buffer is too small, or because the second malloc() failed), then the next call to session_free() is going to call free() on invalid pointers. This bug is fixed in this commit by always setting the pointers to NULL right after they've been read from the serialised state, so that the invalid values can never be used. (An alternative would be to NULL-ify them when writing, which was rejected mostly because we need to do it when reading anyway (as the consequences of free(invalid) are too severe to take any risk), so doing it when writing as well is redundant and a waste of code size.) Also, while thinking about what happens in case of errors, it became apparent to me that it was bad practice to leave the session structure in an half-initialised state and rely on the caller to call session_free(), so this commit also ensures we always clear the structure when loading failed.