Array::CopyFrom and InPlaceVector::TryCopyFrom: do not allow in to alias this.

Also, fix one "theoretical" use-after-free in tls13_enc that happens in
the tls13_rotate_traffic_key -> tls13_set_traffic_key call chain by
resetting the secret to the same secret we already have. This reads the
secret after having called std::destroy_n() on it.

This is not detected by asan and msan, as the std::destroy_n() call
compiles to nothing when the declared type is a built-in integer type
like here (uint8_t). At the same time this means that it also is
harmless - at least for now, as long as all types this happens with are
POD and as long as std::destroy_n() will keep doing nothing on them.

Issue found by asking #Gemini to analyze the code.

Change-Id: I65ef3b45a6fd3b394f85c79a658dfc2e6a6a6964
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/90987
Commit-Queue: Xiangfei Ding <xfding@google.com>
Reviewed-by: Xiangfei Ding <xfding@google.com>

Author: divVerent

crypto/internal.h

@@ -186,6 +186,13 @@ inline int buffers_alias(const void *a, size_t a_bytes, const void *b,
   return a_u + a_bytes > b_u && b_u + b_bytes > a_u;
 }
 
+// spans_alias returns one if |a| and |b| alias, and zero otherwise.
+template <typename T>
+inline int spans_alias(Span<const T> a, Span<const T> b) {
+  return buffers_alias(a.data(), a.size() * sizeof(T), b.data(),
+                       b.size() * sizeof(T));
+}
+
 // align_pointer returns |ptr|, advanced to |alignment|. |alignment| must be a
 // power of two, and |ptr| must have at least |alignment - 1| bytes of scratch
 // space.

crypto/mem_internal.h

@@ -265,7 +265,10 @@ class Array {
 
   // CopyFrom replaces the array with a newly-allocated copy of |in|. It returns
   // true on success and false on error.
+  //
+  // |in| may not alias |this|.
   [[nodiscard]] bool CopyFrom(Span<const T> in) {
+    BSSL_CHECK(!spans_alias(MakeConstSpan(*this), in));
     if (!InitUninitialized(in.size())) {
       return false;
     }
@@ -585,7 +588,10 @@ class InplaceVector {
 
   // TryCopyFrom sets the vector to a copy of |in| and returns true, or returns
   // false if |in| is too large.
+  //
+  // |in| may not alias |this|.
   [[nodiscard]] bool TryCopyFrom(Span<const T> in) {
+    BSSL_CHECK(!spans_alias(MakeConstSpan(*this), in));
     if (in.size() > capacity()) {
       return false;
     }

ssl/tls13_enc.cc

@@ -399,17 +399,17 @@ bool tls13_derive_application_secrets(SSL_HANDSHAKE *hs) {
 static const char kTLS13LabelApplicationTraffic[] = "traffic upd";
 
 bool tls13_rotate_traffic_key(SSL *ssl, enum evp_aead_direction_t direction) {
-  Span<uint8_t> secret = direction == evp_aead_open
-                             ? Span(ssl->s3->read_traffic_secret)
-                             : Span(ssl->s3->write_traffic_secret);
+  InplaceVector<uint8_t, SSL_MAX_MD_SIZE> secret(
+      direction == evp_aead_open ? ssl->s3->read_traffic_secret
+                                 : ssl->s3->write_traffic_secret);
 
   const SSL_SESSION *session = SSL_get_session(ssl);
   const EVP_MD *digest = ssl_session_get_digest(session);
-  return hkdf_expand_label(secret, digest, secret,
+  return hkdf_expand_label(Span(secret), digest, secret,
                            kTLS13LabelApplicationTraffic, {},
                            SSL_is_dtls(ssl)) &&
          tls13_set_traffic_key(ssl, ssl_encryption_application, direction,
-                               session, secret);
+                               session, Span(secret));
 }
 
 static const char kTLS13LabelResumption[] = "res master";