On Tue, Sep 06, 2022 at 09:41:37AM +0100, Daniel P. Berrangé wrote:
> The LUKS spec requires that header strings are NUL-terminated, and our
> code relies on that. Protect against maliciously crafted headers by
> adding validation.
>
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
> crypto/block-luks.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index f62be6836b..27d1b34c1d 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -554,6 +554,24 @@ qcrypto_block_luks_check_header(const QCryptoBlockLUKS
> *luks, Error **errp)
> return -1;
> }
>
> + if (!memchr(luks->header.cipher_name, '\0',
> + sizeof(luks->header.cipher_name))) {
> + error_setg(errp, "LUKS header cipher name is not NUL terminated");
> + return -1;
> + }
> +
> + if (!memchr(luks->header.cipher_mode, '\0',
> + sizeof(luks->header.cipher_mode))) {
> + error_setg(errp, "LUKS header cipher mode is not NUL terminated");
> + return -1;
> + }
> +
> + if (!memchr(luks->header.hash_spec, '\0',
> + sizeof(luks->header.hash_spec))) {
> + error_setg(errp, "LUKS header hash spec is not NUL terminated");
> + return -1;
> + }
> +
> /* Check all keyslots for corruption */
> for (i = 0 ; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS ; i++) {
I think this was the error I originally wrote to you about, and I
think it's the most important fix because non-terminated strings seem
(possibly) exploitable.
FWIW nbdkit does this which is slightly different:
char cipher_name[33], cipher_mode[33], hash_spec[33];
/* Copy the header fields locally and ensure they are \0 terminated. */
memcpy (cipher_name, h->phdr.cipher_name, 32);
cipher_name[32] = 0;
memcpy (cipher_mode, h->phdr.cipher_mode, 32);
cipher_mode[32] = 0;
memcpy (hash_spec, h->phdr.hash_spec, 32);
hash_spec[32] = 0;
Anyway the change above looks good so:
Reviewed-by: Richard W.M. Jones <rjo...@redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
