Fix some problems found by Coverity analysis.

This commit is contained in:
Milan Broz
2017-09-27 10:18:38 +02:00
parent c1c541642e
commit 3ccbb8fe84
9 changed files with 40 additions and 35 deletions

View File

@@ -406,7 +406,8 @@ static int cipher_c2dm(const char *org_c, const char *org_i, unsigned tag_size,
i = sscanf(tmp, "%" CLENS "[^-]-%" CLENS "s", mode, iv); i = sscanf(tmp, "%" CLENS "[^-]-%" CLENS "s", mode, iv);
if (i == 1) { if (i == 1) {
strncpy(iv, mode, CLEN); memset(iv, 0, sizeof(iv));
strncpy(iv, mode, sizeof(iv) - 1);
*mode = '\0'; *mode = '\0';
if (snprintf(capi, sizeof(capi), "%s", cipher) < 0) if (snprintf(capi, sizeof(capi), "%s", cipher) < 0)
return -EINVAL; return -EINVAL;
@@ -495,7 +496,8 @@ static int cipher_dm2c(char **org_c, char **org_i, const char *c_dm, const char
return -ENOMEM; return -ENOMEM;
} else } else
*org_i = NULL; *org_i = NULL;
strncpy(capi, tmp, sizeof(capi)); memset(capi, 0, sizeof(capi));
strncpy(capi, tmp, sizeof(capi) - 1);
} }
i = sscanf(capi, "%" CLENS "[^(](%" CLENS "[^)])", mode, cipher); i = sscanf(capi, "%" CLENS "[^(](%" CLENS "[^)])", mode, cipher);
@@ -1426,9 +1428,10 @@ static int _dm_query_crypt(uint32_t get_flags,
dmd->flags |= CRYPT_ACTIVATE_SUBMIT_FROM_CRYPT_CPUS; dmd->flags |= CRYPT_ACTIVATE_SUBMIT_FROM_CRYPT_CPUS;
else if (sscanf(arg, "integrity:%u:", &val) == 1) { else if (sscanf(arg, "integrity:%u:", &val) == 1) {
dmd->u.crypt.tag_size = val; dmd->u.crypt.tag_size = val;
rintegrity = strchr(arg + strlen("integrity:"), ':') + 1; rintegrity = strchr(arg + strlen("integrity:"), ':');
if (!rintegrity) if (!rintegrity)
goto err; goto err;
rintegrity++;
} else if (sscanf(arg, "sector_size:%u", &val) == 1) { } else if (sscanf(arg, "sector_size:%u", &val) == 1) {
dmd->u.crypt.sector_size = val; dmd->u.crypt.sector_size = val;
} else /* unknown option */ } else /* unknown option */
@@ -1825,7 +1828,7 @@ static int _dm_query_integrity(uint32_t get_flags,
dmd->u.integrity.sector_size = val; dmd->u.integrity.sector_size = val;
else if (sscanf(arg, "buffer_sectors:%u", &val) == 1) else if (sscanf(arg, "buffer_sectors:%u", &val) == 1)
dmd->u.integrity.buffer_sectors = val; dmd->u.integrity.buffer_sectors = val;
else if (!strncmp(arg, "internal_hash:", 14)) { else if (!strncmp(arg, "internal_hash:", 14) && !integrity) {
str = &arg[14]; str = &arg[14];
arg = strsep(&str, ":"); arg = strsep(&str, ":");
integrity = strdup(arg); integrity = strdup(arg);
@@ -1855,7 +1858,7 @@ static int _dm_query_integrity(uint32_t get_flags,
if (r < 0) if (r < 0)
goto err; goto err;
} }
} else if (!strncmp(arg, "journal_crypt:", 14)) { } else if (!strncmp(arg, "journal_crypt:", 14) && !journal_crypt) {
str = &arg[14]; str = &arg[14];
arg = strsep(&str, ":"); arg = strsep(&str, ":");
journal_crypt = strdup(arg); journal_crypt = strdup(arg);
@@ -1863,7 +1866,7 @@ static int _dm_query_integrity(uint32_t get_flags,
r = -ENOMEM; r = -ENOMEM;
goto err; goto err;
} }
} else if (!strncmp(arg, "journal_mac:", 12)) { } else if (!strncmp(arg, "journal_mac:", 12) && !journal_integrity) {
str = &arg[12]; str = &arg[12];
arg = strsep(&str, ":"); arg = strsep(&str, ":");
journal_integrity = strdup(arg); journal_integrity = strdup(arg);

View File

@@ -76,7 +76,7 @@ static int hdr_checksum_calculate(const char *alg, struct luks2_hdr_disk *hdr_di
struct crypt_hash *hd = NULL; struct crypt_hash *hd = NULL;
int r; int r;
if (crypt_hash_init(&hd, alg)) if (crypt_hash_size(alg) <= 0 || crypt_hash_init(&hd, alg))
return -EINVAL; return -EINVAL;
/* Binary header, csum zeroed. */ /* Binary header, csum zeroed. */
@@ -102,6 +102,9 @@ static int hdr_checksum_check(const char *alg, struct luks2_hdr_disk *hdr_disk,
struct luks2_hdr_disk hdr_tmp; struct luks2_hdr_disk hdr_tmp;
int r; int r;
if (crypt_hash_size(alg) <= 0)
return -EINVAL;
/* Copy header and zero checksum. */ /* Copy header and zero checksum. */
memcpy(&hdr_tmp, hdr_disk, LUKS2_HDR_BIN_LEN); memcpy(&hdr_tmp, hdr_disk, LUKS2_HDR_BIN_LEN);
memset(&hdr_tmp.csum, 0, sizeof(hdr_tmp.csum)); memset(&hdr_tmp.csum, 0, sizeof(hdr_tmp.csum));
@@ -576,7 +579,7 @@ int LUKS2_disk_hdr_read(struct crypt_device *cd, struct luks2_hdr *hdr,
state_hdr1 = HDR_OBSOLETE; state_hdr1 = HDR_OBSOLETE;
} }
/* check header with keyslots fit the device */ /* check header with keyslots to fit the device */
if (state_hdr1 == HDR_OK) if (state_hdr1 == HDR_OK)
hdr_size = LUKS2_hdr_and_areas_size(jobj_hdr1); hdr_size = LUKS2_hdr_and_areas_size(jobj_hdr1);
else if (state_hdr2 == HDR_OK) else if (state_hdr2 == HDR_OK)
@@ -641,16 +644,17 @@ int LUKS2_disk_hdr_read(struct crypt_device *cd, struct luks2_hdr *hdr,
hdr_from_disk(&hdr_disk1, &hdr_disk2, hdr, 0); hdr_from_disk(&hdr_disk1, &hdr_disk2, hdr, 0);
hdr->jobj = jobj_hdr1; hdr->jobj = jobj_hdr1;
json_object_put(jobj_hdr2); json_object_put(jobj_hdr2);
return 0;
} else if (state_hdr2 == HDR_OK) { } else if (state_hdr2 == HDR_OK) {
hdr_from_disk(&hdr_disk2, &hdr_disk1, hdr, 1); hdr_from_disk(&hdr_disk2, &hdr_disk1, hdr, 1);
hdr->jobj = jobj_hdr2; hdr->jobj = jobj_hdr2;
json_object_put(jobj_hdr1); json_object_put(jobj_hdr1);
return 0;
} }
r = (state_hdr1 == HDR_FAIL_IO || state_hdr2 == HDR_FAIL_IO) ? -EIO : -EINVAL; /*
* FIXME: should this fail? At least one header was read correctly.
* r = (state_hdr1 == HDR_FAIL_IO || state_hdr2 == HDR_FAIL_IO) ? -EIO : -EINVAL;
*/
return 0;
err: err:
log_dbg("LUKS2 header read failed (%d).", r); log_dbg("LUKS2 header read failed (%d).", r);

View File

@@ -182,7 +182,7 @@ int LUKS2_generate_hdr(
jobj_segment = json_object_new_object(); jobj_segment = json_object_new_object();
json_object_object_add(jobj_segment, "type", json_object_new_string("crypt")); json_object_object_add(jobj_segment, "type", json_object_new_string("crypt"));
if (detached_metadata_device) if (detached_metadata_device)
offset = alignPayload * sector_size; offset = (uint64_t)alignPayload * sector_size;
else { else {
//FIXME //FIXME
//offset = size_round_up(areas[7].offset + areas[7].length, alignPayload * SECTOR_SIZE); //offset = size_round_up(areas[7].offset + areas[7].length, alignPayload * SECTOR_SIZE);

View File

@@ -941,6 +941,7 @@ int LUKS2_hdr_backup(struct crypt_device *cd, struct luks2_hdr *hdr,
if (r) { if (r) {
log_err(cd, _("Failed to acquire read lock on device %s.\n"), log_err(cd, _("Failed to acquire read lock on device %s.\n"),
device_path(crypt_metadata_device(cd))); device_path(crypt_metadata_device(cd)));
crypt_safe_free(buffer);
return r; return r;
} }
@@ -969,7 +970,6 @@ int LUKS2_hdr_backup(struct crypt_device *cd, struct luks2_hdr *hdr,
log_err(cd, _("Requested header backup file %s already exists.\n"), backup_file); log_err(cd, _("Requested header backup file %s already exists.\n"), backup_file);
else else
log_err(cd, _("Cannot create header backup file %s.\n"), backup_file); log_err(cd, _("Cannot create header backup file %s.\n"), backup_file);
close(devfd);
crypt_safe_free(buffer); crypt_safe_free(buffer);
return -EINVAL; return -EINVAL;
} }

View File

@@ -126,6 +126,7 @@ static int luks2_decrypt_from_storage(char *dst, size_t dstLength,
if (r) { if (r) {
log_err(cd, _("Failed to acquire read lock on device %s.\n"), log_err(cd, _("Failed to acquire read lock on device %s.\n"),
device_path(device)); device_path(device));
crypt_storage_destroy(s);
return r; return r;
} }
@@ -143,15 +144,13 @@ static int luks2_decrypt_from_storage(char *dst, size_t dstLength,
device_read_unlock(device); device_read_unlock(device);
if (r) {
log_err(cd, _("IO error while decrypting keyslot.\n"));
return r;
}
/* Decrypt buffer */ /* Decrypt buffer */
r = crypt_storage_decrypt(s, 0, dstLength / SECTOR_SIZE, dst); if (!r)
crypt_storage_destroy(s); r = crypt_storage_decrypt(s, 0, dstLength / SECTOR_SIZE, dst);
else
log_err(cd, _("IO error while decrypting keyslot.\n"));
crypt_storage_destroy(s);
return r; return r;
#endif #endif
} }
@@ -314,9 +313,10 @@ static int luks2_keyslot_get_key(struct crypt_device *cd,
if (!json_object_object_get_ex(jobj_kdf, "salt", &jobj2)) if (!json_object_object_get_ex(jobj_kdf, "salt", &jobj2))
return -EINVAL; return -EINVAL;
salt_len = LUKS_SALTSIZE; salt_len = LUKS_SALTSIZE;
base64_decode(json_object_get_string(jobj2), if (!base64_decode(json_object_get_string(jobj2),
json_object_get_string_len(jobj2), json_object_get_string_len(jobj2),
salt, &salt_len); salt, &salt_len))
return -EINVAL;
if (salt_len != LUKS_SALTSIZE) if (salt_len != LUKS_SALTSIZE)
return -EINVAL; return -EINVAL;

View File

@@ -138,7 +138,7 @@ static int json_luks1_segment(const struct luks_phdr *hdr_v1, struct json_object
json_object_object_add(segment_obj, "type", field); json_object_object_add(segment_obj, "type", field);
/* offset field */ /* offset field */
number = hdr_v1->payloadOffset * SECTOR_SIZE; number = (uint64_t)hdr_v1->payloadOffset * SECTOR_SIZE;
field = json_object_new_string(uint64_to_str(num, sizeof(num), &number)); field = json_object_new_string(uint64_to_str(num, sizeof(num), &number));
if (!field) { if (!field) {
@@ -442,6 +442,7 @@ static int move_keyslot_areas(struct crypt_device *cd, off_t offset_from,
devfd = device_open(device, O_RDWR); devfd = device_open(device, O_RDWR);
if (devfd == -1) { if (devfd == -1) {
log_dbg("Cannot open device %s.", device_path(device)); log_dbg("Cannot open device %s.", device_path(device));
free(buf);
return -EIO; return -EIO;
} }
@@ -521,7 +522,7 @@ int LUKS2_luks1_to_luks2(struct crypt_device *cd, struct luks_phdr *hdr1, struct
strncpy(hdr2->checksum_alg, "sha256", LUKS2_CHECKSUM_ALG_L); strncpy(hdr2->checksum_alg, "sha256", LUKS2_CHECKSUM_ALG_L);
crypt_random_get(cd, (char*)hdr2->salt1, sizeof(hdr2->salt1), CRYPT_RND_SALT); crypt_random_get(cd, (char*)hdr2->salt1, sizeof(hdr2->salt1), CRYPT_RND_SALT);
crypt_random_get(cd, (char*)hdr2->salt2, sizeof(hdr2->salt2), CRYPT_RND_SALT); crypt_random_get(cd, (char*)hdr2->salt2, sizeof(hdr2->salt2), CRYPT_RND_SALT);
strncpy(hdr2->uuid, crypt_get_uuid(cd), LUKS2_UUID_L); strncpy(hdr2->uuid, crypt_get_uuid(cd), LUKS2_UUID_L-1); /* UUID should be max 36 chars */
hdr2->jobj = jobj; hdr2->jobj = jobj;
/* /*
@@ -769,7 +770,7 @@ int LUKS2_luks2_to_luks1(struct crypt_device *cd, struct luks2_hdr *hdr2, struct
/* FIXME: LUKS1 requires offset == 0 || offset >= luks1_hdr_size */ /* FIXME: LUKS1 requires offset == 0 || offset >= luks1_hdr_size */
hdr1->payloadOffset = offset; hdr1->payloadOffset = offset;
strncpy(hdr1->uuid, hdr2->uuid, UUID_STRING_L); strncpy(hdr1->uuid, hdr2->uuid, UUID_STRING_L - 1); /* max 36 chars */
memcpy(hdr1->magic, luksMagic, LUKS_MAGIC_L); memcpy(hdr1->magic, luksMagic, LUKS_MAGIC_L);

View File

@@ -501,10 +501,8 @@ static int device_info(struct crypt_device *cd,
int fd = -1, r, flags = 0, real_readonly; int fd = -1, r, flags = 0, real_readonly;
uint64_t real_size; uint64_t real_size;
if (!device) { if (!device)
r = -ENOTBLK; return -ENOTBLK;
goto out;
}
real_readonly = 0; real_readonly = 0;
real_size = 0; real_size = 0;

View File

@@ -169,6 +169,7 @@ static void release_lock_handle(struct crypt_lock_handle *h)
char res[PATH_MAX]; char res[PATH_MAX];
struct stat buf_a, buf_b; struct stat buf_a, buf_b;
/* coverity[toctou] */
if (S_ISBLK(h->mode) && /* was it block device */ if (S_ISBLK(h->mode) && /* was it block device */
!flock(h->flock_fd, LOCK_EX | LOCK_NB) && /* lock to drop the file */ !flock(h->flock_fd, LOCK_EX | LOCK_NB) && /* lock to drop the file */
!resource_by_devno(res, sizeof(res), h->devno, 1) && /* acquire lock resource name */ !resource_by_devno(res, sizeof(res), h->devno, 1) && /* acquire lock resource name */

View File

@@ -106,8 +106,7 @@ static int _read_keys(char **integrity_key, struct crypt_params_integrity *param
if (opt_journal_integrity_key_file) { if (opt_journal_integrity_key_file) {
r = _read_mk(opt_journal_integrity_key_file, &journal_integrity_key, opt_journal_integrity_key_size); r = _read_mk(opt_journal_integrity_key_file, &journal_integrity_key, opt_journal_integrity_key_size);
if (r < 0) { if (r < 0) {
crypt_safe_free(*integrity_key); crypt_safe_free(int_key);
*integrity_key = NULL;
return r; return r;
} }
params->journal_integrity_key = journal_integrity_key; params->journal_integrity_key = journal_integrity_key;
@@ -117,8 +116,7 @@ static int _read_keys(char **integrity_key, struct crypt_params_integrity *param
if (opt_journal_crypt_key_file) { if (opt_journal_crypt_key_file) {
r = _read_mk(opt_journal_crypt_key_file, &journal_crypt_key, opt_journal_crypt_key_size); r = _read_mk(opt_journal_crypt_key_file, &journal_crypt_key, opt_journal_crypt_key_size);
if (r < 0) { if (r < 0) {
crypt_safe_free(*integrity_key); crypt_safe_free(int_key);
*integrity_key = NULL;
crypt_safe_free(journal_integrity_key); crypt_safe_free(journal_integrity_key);
return r; return r;
} }