fix(hotp): bound Non-Secure key size before copy into K[]

register_shared_key() copied params[0].memref.size bytes into the fixed
64-byte secure buffer K[] without bounding the length. That size is
supplied verbatim by the Non-Secure caller, so any NS caller could
overflow a secure .bss buffer with attacker-controlled data.

This is not covered by the pointer-attribution hardening in 27e314c:
nsec_check() proves the source range is Non-Secure resident, which is a
different property from "the source fits in the destination". A
legitimate 4 KB Non-Secure buffer passes that check exactly as designed
and is then copied into 64 bytes.

The correct bounds check already existed in this file (MIN_KEY_SIZE /
MAX_KEY_SIZE in hmac_sha1()) but ran on the GET_HOTP path, a later and
separate invocation, long after the corruption. Move the identical
rejection ahead of the copy. Reject rather than truncate, matching
apps/aes/ta/aes_ta.c: a silently shortened HMAC key would be a
cryptographic defect of its own. Not a compatibility break, since such a
key already failed later at hmac_sha1().

Two further defects in the same code:
 - DMSG traced K with "%s": it printed the HMAC shared secret to the
   console, and a key of exactly sizeof(K) leaves no NUL for %s to stop
   at, so it also over-read past the array. Trace only the length.
 - get_hotp() called truncate_() without checking res from hmac_sha1(),
   truncating uninitialised stack when the HMAC failed.

Verified under QEMU (mps2-an505, cortex-m33): sizes 4096, 65 and 4 are
now rejected with TEE_ERROR_BAD_PARAMETERS, the 20-byte RFC4226 key still
registers and GET_HOTP still yields the specified value 755224. With this
patch reverted the 4096-byte request returns TEE_SUCCESS and the secure
world then stops responding.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: 2 people

apps/hotp/ta/hotp_ta.c

@@ -144,12 +144,36 @@ static TEE_Result register_shared_key(uint32_t param_types, TEE_Param params[4])
 		return TEE_ERROR_BAD_PARAMETERS;
 	}
 
+	/*
+	 * params[0] comes from the Non-Secure world: both the buffer pointer and
+	 * the size are attacker-controlled. The secure-world boundary only
+	 * guarantees the source range is Non-Secure resident -- it says nothing
+	 * about how much of it fits in K, so the size must be bounded here
+	 * before it is used as a memcpy() length. Reject rather than truncate: a
+	 * silently shortened HMAC key would be a cryptographic bug of its own.
+	 */
+	if (!params[0].memref.buffer)
+		return TEE_ERROR_BAD_PARAMETERS;
+
+	if (params[0].memref.size < MIN_KEY_SIZE ||
+	    params[0].memref.size > sizeof(K)) {
+		EMSG("Bad key size %u, expect %u..%u bytes",
+		     (unsigned int)params[0].memref.size,
+		     (unsigned int)MIN_KEY_SIZE, (unsigned int)sizeof(K));
+		return TEE_ERROR_BAD_PARAMETERS;
+	}
+
 	memset(counter, 0, sizeof(counter));
 	memset(K, 0, sizeof(K));
 	memcpy(K, params[0].memref.buffer, params[0].memref.size);
 
 	K_len = params[0].memref.size;
-	DMSG("Got shared key %s (%u bytes).", K, params[0].memref.size);
+	/*
+	 * Do not trace K itself: it is the HMAC shared secret, and a key of
+	 * exactly sizeof(K) bytes leaves no NUL for the "%s" to stop at.
+	 */
+	DMSG("Registered shared key (%u bytes).",
+	     (unsigned int)params[0].memref.size);
 
 	return res;
 }
@@ -173,6 +197,10 @@ static TEE_Result get_hotp(uint32_t param_types, TEE_Param params[4])
 	}
 
 	res = hmac_sha1(K, K_len, counter, sizeof(counter), mac, &mac_len);
+	if (res != TEE_SUCCESS) {
+		EMSG("hmac_sha1 failed: 0x%x", res);
+		return res;
+	}
 
 	/* Increment the counter. */
 	for (i = sizeof(counter) - 1; i >= 0; i--) {