[crypto] Check length before decrypting data

Using a CBC-mode cipher with a length that is not a multiple of the
block size will cause an uncontrollable out-of-bounds write of the
entire address space, resulting in a guaranteed crash.  There is no
way to cause the CBC cipher to return before overwriting the entire
address space with pseudorandom data, and so this could only be used
as a denial-of-service attack.

Almost all cipher call sites already validate the length against the
block size.  Add the relevant checks to the two call sites that do not
already do so.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/net/peerblk.c

@@ -1125,8 +1125,15 @@ static void peerblk_decrypt ( struct peerdist_block *peerblk ) {
 	void *data;
 	int rc;
 
-	/* Sanity check */
+	/* Sanity checks */
 	assert ( ( PEERBLK_DECRYPT_CHUNKSIZE % cipher->blocksize ) == 0 );
+	if ( peerblk->cipher_remaining & ( cipher->blocksize - 1 ) ) {
+		DBGC ( peerblk, "PEERBLK %p %d.%d has invalid length %zd\n",
+		       peerblk, peerblk->segment, peerblk->block,
+		       peerblk->cipher_remaining );
+		rc = -EINVAL;
+		goto err_blocksize;
+	}
 
 	/* Get the underlying data transfer buffer */
 	xferbuf = xfer_buffer ( &peerblk->xfer );
@@ -1197,6 +1204,7 @@ static void peerblk_decrypt ( struct peerdist_block *peerblk ) {
 	free ( data );
  err_alloc_data:
  err_xfer_buffer:
+ err_blocksize:
 	peerblk_done ( peerblk, rc );
 }
 

src/net/tls.c

@@ -82,6 +82,10 @@ FILE_SECBOOT ( PERMITTED );
 #define EINFO_EINVAL_INNER						\
 	__einfo_uniqify ( EINFO_EINVAL, 0x10,				\
 			  "Invalid inner plaintext" )
+#define EINVAL_BLOCK __einfo_error ( EINFO_EINVAL_BLOCK )
+#define EINFO_EINVAL_BLOCK						\
+	__einfo_uniqify ( EINFO_EINVAL, 0x11,				\
+			  "Invalid block cipher size" )
 #define EIO_ALERT __einfo_error ( EINFO_EIO_ALERT )
 #define EINFO_EIO_ALERT							\
 	__einfo_uniqify ( EINFO_EIO, 0x01,				\
@@ -3897,6 +3901,14 @@ static int tls_new_ciphertext ( struct tls_connection *tls,
 	len -= cipher->authsize;
 	auth = last->tail;
 
+	/* Check that overall length is a multiple of the cipher blocksize */
+	assert ( ( TLS_RX_BUFSIZE % cipher->blocksize ) == 0 );
+	if ( iob_len ( last ) & ( cipher->blocksize - 1 ) ) {
+		DBGC ( tls, "TLS %p invalid received length %zd\n",
+		       tls, len );
+		return -EINVAL_BLOCK;
+	}
+
 	/* Set initialisation vector */
 	if ( ( rc = cipher_setiv ( cipher, pipe->ctx, &iv,
 				   sizeof ( iv ) ) ) != 0 ) {