From 170161b9b6aaae023b3deba0bbd20a7135cbf70b Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Tue, 6 Dec 2022 15:06:34 +0100 Subject: [PATCH] Free all possible allocated params if crypt_load() fails. If format load fails in some intermediate step, the internal params struct can contain already set values. While context is set still to none type, it can cause segfault in releasing active_name. (Found by fuzzing target processing crypt_load.) --- lib/setup.c | 94 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/lib/setup.c b/lib/setup.c index 9b7026c1..125cd833 100644 --- a/lib/setup.c +++ b/lib/setup.c @@ -920,11 +920,13 @@ static int _crypt_load_tcrypt(struct crypt_device *cd, struct crypt_params_tcryp cd->u.tcrypt.params.veracrypt_pim = 0; if (r < 0) - return r; + goto out; if (!cd->type && !(cd->type = strdup(CRYPT_TCRYPT))) - return -ENOMEM; - + r = -ENOMEM; +out: + if (r < 0) + crypt_free_type(cd, CRYPT_TCRYPT); return r; } @@ -945,14 +947,11 @@ static int _crypt_load_verity(struct crypt_device *cd, struct crypt_params_verit r = VERITY_read_sb(cd, sb_offset, &cd->u.verity.uuid, &cd->u.verity.hdr); if (r < 0) - return r; + goto out; if (!cd->type && !(cd->type = strdup(CRYPT_VERITY))) { - free(CONST_CAST(void*)cd->u.verity.hdr.hash_name); - free(CONST_CAST(void*)cd->u.verity.hdr.salt); - free(cd->u.verity.uuid); - crypt_safe_memzero(&cd->u.verity.hdr, sizeof(cd->u.verity.hdr)); - return -ENOMEM; + r = -ENOMEM; + goto out; } if (params) @@ -960,21 +959,25 @@ static int _crypt_load_verity(struct crypt_device *cd, struct crypt_params_verit /* Hash availability checked in sb load */ cd->u.verity.root_hash_size = crypt_hash_size(cd->u.verity.hdr.hash_name); - if (cd->u.verity.root_hash_size > 4096) - return -EINVAL; + if (cd->u.verity.root_hash_size > 4096) { + r = -EINVAL; + goto out; + } if (params && params->data_device && (r = crypt_set_data_device(cd, params->data_device)) < 0) - return r; + goto out; if (params && params->fec_device) { r = device_alloc(cd, &cd->u.verity.fec_device, params->fec_device); if (r < 0) - return r; + goto out; cd->u.verity.hdr.fec_area_offset = params->fec_area_offset; cd->u.verity.hdr.fec_roots = params->fec_roots; } - +out: + if (r < 0) + crypt_free_type(cd, CRYPT_VERITY); return r; } @@ -989,45 +992,49 @@ static int _crypt_load_integrity(struct crypt_device *cd, r = INTEGRITY_read_sb(cd, &cd->u.integrity.params, &cd->u.integrity.sb_flags); if (r < 0) - return r; + goto out; // FIXME: add checks for fields in integrity sb vs params + r = -ENOMEM; if (params) { cd->u.integrity.params.journal_watermark = params->journal_watermark; cd->u.integrity.params.journal_commit_time = params->journal_commit_time; cd->u.integrity.params.buffer_sectors = params->buffer_sectors; - // FIXME: check ENOMEM - if (params->integrity) - cd->u.integrity.params.integrity = strdup(params->integrity); + if (params->integrity && + !(cd->u.integrity.params.integrity = strdup(params->integrity))) + goto out; cd->u.integrity.params.integrity_key_size = params->integrity_key_size; - if (params->journal_integrity) - cd->u.integrity.params.journal_integrity = strdup(params->journal_integrity); - if (params->journal_crypt) - cd->u.integrity.params.journal_crypt = strdup(params->journal_crypt); + if (params->journal_integrity && + !(cd->u.integrity.params.journal_integrity = strdup(params->journal_integrity))) + goto out; + if (params->journal_crypt && + !(cd->u.integrity.params.journal_crypt = strdup(params->journal_crypt))) + goto out; if (params->journal_crypt_key) { cd->u.integrity.journal_crypt_key = crypt_alloc_volume_key(params->journal_crypt_key_size, params->journal_crypt_key); if (!cd->u.integrity.journal_crypt_key) - return -ENOMEM; + goto out; } if (params->journal_integrity_key) { cd->u.integrity.journal_mac_key = crypt_alloc_volume_key(params->journal_integrity_key_size, params->journal_integrity_key); if (!cd->u.integrity.journal_mac_key) - return -ENOMEM; + goto out; } } - if (!cd->type && !(cd->type = strdup(CRYPT_INTEGRITY))) { - free(CONST_CAST(void*)cd->u.integrity.params.integrity); - return -ENOMEM; - } - - return 0; + if (!cd->type && !(cd->type = strdup(CRYPT_INTEGRITY))) + goto out; + r = 0; +out: + if (r < 0) + crypt_free_type(cd, CRYPT_INTEGRITY); + return r; } static int _crypt_load_bitlk(struct crypt_device *cd) @@ -1040,20 +1047,25 @@ static int _crypt_load_bitlk(struct crypt_device *cd) r = BITLK_read_sb(cd, &cd->u.bitlk.params); if (r < 0) - return r; + goto out; if (asprintf(&cd->u.bitlk.cipher_spec, "%s-%s", cd->u.bitlk.params.cipher, cd->u.bitlk.params.cipher_mode) < 0) { cd->u.bitlk.cipher_spec = NULL; - return -ENOMEM; + r = -ENOMEM; + goto out; } - if (!cd->type && !(cd->type = strdup(CRYPT_BITLK))) - return -ENOMEM; + if (!cd->type && !(cd->type = strdup(CRYPT_BITLK))) { + r = -ENOMEM; + goto out; + } device_set_block_size(crypt_data_device(cd), cd->u.bitlk.params.sector_size); - - return 0; +out: + if (r < 0) + crypt_free_type(cd, CRYPT_BITLK); + return r; } static int _crypt_load_fvault2(struct crypt_device *cd) @@ -1066,12 +1078,14 @@ static int _crypt_load_fvault2(struct crypt_device *cd) r = FVAULT2_read_metadata(cd, &cd->u.fvault2.params); if (r < 0) - return r; + goto out; if (!cd->type && !(cd->type = strdup(CRYPT_FVAULT2))) - return -ENOMEM; - - return 0; + r = -ENOMEM; +out: + if (r < 0) + crypt_free_type(cd, CRYPT_FVAULT2); + return r; } int crypt_load(struct crypt_device *cd,