Fix various coverity issues.

Mostly INTEGER_OVERFLOW (CWE-190).
This commit is contained in:
Ondrej Kozina
2024-05-02 11:50:45 +02:00
parent 33e26be58b
commit bede116926
8 changed files with 78 additions and 40 deletions

View File

@@ -261,7 +261,7 @@ int crypt_storage_init(struct crypt_storage **ctx,
}
s->sector_size = sector_size;
s->iv_shift = large_iv ? int_log2(sector_size) - SECTOR_SHIFT : 0;
s->iv_shift = large_iv ? (unsigned)int_log2(sector_size) - SECTOR_SHIFT : 0;
*ctx = s;
return 0;

View File

@@ -230,6 +230,7 @@ static size_t utf16_encode_unichar(char16_t *out, char32_t c)
return 1;
case 0x10000U ... 0x10ffffU:
/* coverity[overflow_const:FALSE] */
c -= 0x10000U;
out[0] = htole16((c >> 10) + 0xd800U);
out[1] = htole16((c & 0x3ffU) + 0xdc00U);

View File

@@ -621,6 +621,10 @@ int LUKS2_luks1_to_luks2(struct crypt_device *cd, struct luks_phdr *hdr1, struct
if (max_size < required_size)
max_size = required_size;
/* fix coverity false positive integer underflow */
if (max_size < 2 * LUKS2_HDR_16K_LEN)
return -EINVAL;
r = json_luks1_object(hdr1, &jobj, max_size - 2 * LUKS2_HDR_16K_LEN);
if (r < 0)
return r;

View File

@@ -1070,7 +1070,7 @@ uint64_t TCRYPT_get_iv_offset(struct crypt_device *cd,
struct tcrypt_phdr *hdr,
struct crypt_params_tcrypt *params)
{
uint64_t iv_offset;
uint64_t iv_offset, partition_offset;
if (params->mode && !strncmp(params->mode, "xts", 3))
iv_offset = TCRYPT_get_data_offset(cd, hdr, params);
@@ -1079,8 +1079,14 @@ uint64_t TCRYPT_get_iv_offset(struct crypt_device *cd,
else
iv_offset = hdr->d.mk_offset / SECTOR_SIZE;
if (params->flags & CRYPT_TCRYPT_SYSTEM_HEADER)
iv_offset += crypt_dev_partition_offset(device_path(crypt_data_device(cd)));
if (params->flags & CRYPT_TCRYPT_SYSTEM_HEADER) {
partition_offset = crypt_dev_partition_offset(device_path(crypt_data_device(cd)));
/* FIXME: we need to deal with overflow sooner */
if (iv_offset > (UINT64_MAX - partition_offset))
iv_offset = UINT64_MAX;
else
iv_offset += partition_offset;
}
return iv_offset;
}

View File

@@ -22,6 +22,7 @@
*/
#include <errno.h>
#include <limits.h>
#include <string.h>
#include <stdlib.h>
#include <stdint.h>
@@ -32,10 +33,9 @@
/* coverity[ -taint_source : arg-1 ] */
static ssize_t _read_buffer(int fd, void *buf, size_t length, volatile int *quit)
{
size_t read_size = 0;
ssize_t r;
ssize_t r, read_size = 0;
if (fd < 0 || !buf)
if (fd < 0 || !buf || length > SSIZE_MAX)
return -EINVAL;
do {
@@ -43,12 +43,13 @@ static ssize_t _read_buffer(int fd, void *buf, size_t length, volatile int *quit
if (r == -1 && errno != EINTR)
return r;
if (r > 0) {
read_size += (size_t)r;
/* coverity[overflow:FALSE] */
read_size += r;
buf = (uint8_t*)buf + r;
}
if (r == 0 || (quit && *quit))
return (ssize_t)read_size;
} while (read_size != length);
return read_size;
} while ((size_t)read_size != length);
return (ssize_t)length;
}
@@ -65,25 +66,25 @@ ssize_t read_buffer_intr(int fd, void *buf, size_t length, volatile int *quit)
static ssize_t _write_buffer(int fd, const void *buf, size_t length, volatile int *quit)
{
size_t write_size = 0;
ssize_t w;
ssize_t w, write_size = 0;
if (fd < 0 || !buf || !length)
if (fd < 0 || !buf || !length || length > SSIZE_MAX)
return -EINVAL;
do {
w = write(fd, buf, length - write_size);
w = write(fd, buf, length - (size_t)write_size);
if (w < 0 && errno != EINTR)
return w;
if (w > 0) {
write_size += (size_t) w;
/* coverity[overflow:FALSE] */
write_size += w;
buf = (const uint8_t*)buf + w;
}
if (w == 0 || (quit && *quit))
return (ssize_t)write_size;
} while (write_size != length);
return write_size;
} while ((size_t)write_size != length);
return (ssize_t)write_size;
return write_size;
}
ssize_t write_buffer(int fd, const void *buf, size_t length)

View File

@@ -158,7 +158,7 @@ static key_serial_t find_key_by_type_and_desc(const char *type, const char *desc
char *newline;
size_t buffer_len = 0;
int n;
ssize_t n;
do {
id = request_key(type, desc, NULL, 0);
@@ -171,7 +171,8 @@ static key_serial_t find_key_by_type_and_desc(const char *type, const char *desc
return 0;
while ((n = read(f, buf + buffer_len, sizeof(buf) - buffer_len - 1)) > 0) {
buffer_len += n;
/* coverity[overflow:FALSE] */
buffer_len += (size_t)n;
buf[buffer_len] = '\0';
newline = strchr(buf, '\n');
while (newline != NULL && buffer_len != 0) {

View File

@@ -109,8 +109,10 @@ static int tools_check_password(const char *password)
static ssize_t read_tty_eol(int fd, char *pass, size_t maxlen)
{
bool eol = false;
size_t read_size = 0;
ssize_t r;
ssize_t r, read_size = 0;
if (maxlen > SSIZE_MAX)
return -EINVAL;
do {
r = read(fd, pass, maxlen - read_size);
@@ -119,12 +121,13 @@ static ssize_t read_tty_eol(int fd, char *pass, size_t maxlen)
if (r >= 0) {
if (!r || pass[r-1] == '\n')
eol = true;
read_size += (size_t)r;
/* coverity[overflow:FALSE] */
read_size += r;
pass = pass + r;
}
} while (!eol && read_size != maxlen);
} while (!eol && (size_t)read_size != maxlen);
return (ssize_t)read_size;
return read_size;
}
/* The pass buffer is zeroed and has trailing \0 already " */

View File

@@ -690,10 +690,13 @@ out:
return r;
}
/* FIXME this function will be replaced with equivalent from lib/utils_io.c */
static ssize_t read_buf(int fd, void *buf, size_t count)
{
size_t read_size = 0;
ssize_t s;
ssize_t s, read_size = 0;
if (count > SSIZE_MAX)
return -EINVAL;
do {
/* This expects that partial read is aligned in buffer */
@@ -701,14 +704,17 @@ static ssize_t read_buf(int fd, void *buf, size_t count)
if (s == -1 && errno != EINTR)
return s;
if (s == 0)
return (ssize_t)read_size;
return read_size;
if (s > 0) {
if (s != (ssize_t)count)
log_dbg("Partial read %zd / %zu.", s, count);
read_size += (size_t)s;
/* FIXME: temporarily silence coverity INTEGER_OVERFLOW (CWE-190) */
if (read_size > SSIZE_MAX - s)
return -1;
read_size += s;
buf = (uint8_t*)buf + s;
}
} while (read_size != count);
} while ((size_t)read_size != count);
return (ssize_t)count;
}
@@ -727,6 +733,10 @@ static int copy_data_forward(struct reenc_ctx *rc, int fd_old, int fd_new,
.device = tools_get_device_name(rc->device, &backing_file)
};
assert(rc);
assert(bytes);
assert(buf);
log_dbg("Reencrypting in forward direction.");
if (lseek(fd_old, rc->device_offset, SEEK_SET) < 0 ||
@@ -746,19 +756,18 @@ static int copy_data_forward(struct reenc_ctx *rc, int fd_old, int fd_new,
if ((rc->device_size - rc->device_offset) < (uint64_t)block_size)
block_size = rc->device_size - rc->device_offset;
s1 = read_buf(fd_old, buf, block_size);
if (s1 < 0 || ((size_t)s1 != block_size &&
(rc->device_offset + s1) != rc->device_size)) {
if (s1 < 0 || ((size_t)s1 != block_size)) {
log_dbg("Read error, expecting %zu, got %zd.",
block_size, s1);
goto out;
}
/* If device_size is forced, never write more than limit */
if ((s1 + rc->device_offset) > rc->device_size)
s1 = rc->device_size - rc->device_offset;
/*
* FIXME: this may fail with shorter write.
* Replace ir with write_buffer from lib/utils_io.c
*/
s2 = write(fd_new, buf, s1);
if (s2 < 0) {
if (s2 < 0 || s2 != s1) {
log_dbg("Write error, expecting %zu, got %zd.",
block_size, s2);
goto out;
@@ -840,6 +849,10 @@ static int copy_data_backward(struct reenc_ctx *rc, int fd_old, int fd_new,
goto out;
}
/*
* FIXME: this may fail with shorter write.
* Replace ir with write_buffer from lib/utils_io.c
*/
s2 = write(fd_new, buf, working_block);
if (s2 < 0) {
log_dbg("Write error, expecting %zu, got %zd.",
@@ -872,6 +885,11 @@ static void zero_rest_of_device(int fd, size_t block_size, void *buf,
{
ssize_t s1, s2;
assert(bytes);
if (*bytes > SSIZE_MAX || !block_size || block_size > SSIZE_MAX)
return;
log_dbg("Zeroing rest of device.");
if (lseek(fd, offset, SEEK_SET) < 0) {
@@ -884,10 +902,14 @@ static void zero_rest_of_device(int fd, size_t block_size, void *buf,
while (!quit && *bytes) {
if (*bytes < (uint64_t)s1)
s1 = *bytes;
s1 = (ssize_t)*bytes;
/*
* FIXME: this may fail with shorter write.
* Replace ir with write_buffer from lib/utils_io.c
*/
s2 = write(fd, buf, s1);
if (s2 != s1) {
if (s2 < 0 || s2 != s1) {
log_dbg("Write error, expecting %zd, got %zd.",
s1, s2);
return;
@@ -898,7 +920,7 @@ static void zero_rest_of_device(int fd, size_t block_size, void *buf,
return;
}
*bytes -= s2;
*bytes -= (uint64_t)s2;
}
}