Skip to content

Commit

Permalink
[misc] fix Coverity warnings
Browse files Browse the repository at this point in the history
* Also use a new if_not_assert() construct where possible.
  • Loading branch information
pbatard committed Jul 17, 2024
1 parent 78608c3 commit ceee3e9
Show file tree
Hide file tree
Showing 24 changed files with 133 additions and 70 deletions.
10 changes: 9 additions & 1 deletion src/badblocks.c
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,11 @@ static unsigned int test_rw(HANDLE hDrive, blk64_t last_block, size_t block_size
cancel_ops = -1;
return 0;
}
if ((first_block * block_size > 1 * PB) || (last_block * block_size > 1 * PB)) {
uprintf("%sDisk is too large\n", bb_prefix);
cancel_ops = -1;
return 0;
}

buffer = allocate_buffer(2 * blocks_at_once * block_size);
if (!buffer) {
Expand Down Expand Up @@ -543,8 +548,11 @@ static unsigned int test_rw(HANDLE hDrive, blk64_t last_block, size_t block_size
for (i=0; i < got; i++) {
if (memcmp(read_buffer + i * block_size,
buffer + i * block_size,
block_size))
block_size)) {
if_not_assert(currently_testing * block_size < 1 * PB)
goto out;
bb_count += bb_output(currently_testing+i-got, CORRUPTION_ERROR);
}
}
if (v_flag > 1)
print_status();
Expand Down
2 changes: 2 additions & 0 deletions src/bled/crc32.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static void crc32init_le(uint32_t *crc32table_le)
crc32table_le[0] = 0;

for (i = 1 << (CRC_LE_BITS - 1); i; i >>= 1) {
// coverity[overflow_const]
crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
for (j = 0; j < 1 << CRC_LE_BITS; j += 2 * i)
crc32table_le[i + j] = crc ^ crc32table_le[j];
Expand Down Expand Up @@ -81,6 +82,7 @@ static void crc32init_be(uint32_t *crc32table_be)
uint32_t crc = 0x80000000;

for (i = 1; i < 1 << CRC_BE_BITS; i <<= 1) {
// coverity[overflow_const]
crc = (crc << 1) ^ ((crc & 0x80000000) ? CRCPOLY_BE : 0);
for (j = 0; j < i; j++)
crc32table_be[i + j] = crc ^ crc32table_be[j];
Expand Down
14 changes: 12 additions & 2 deletions src/bled/decompress_gunzip.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ static unsigned fill_bitbuffer(STATE_PARAM unsigned bitbuffer, unsigned *current
bytebuffer_offset++;
*current += 8;
}
// coverity[return_overflow]
return bitbuffer;
}

Expand Down Expand Up @@ -661,7 +662,7 @@ static NOINLINE int inflate_codes(STATE_PARAM_ONLY)


/* called once from inflate_block */
static void inflate_stored_setup(STATE_PARAM int my_n, int my_b, int my_k)
static void inflate_stored_setup(STATE_PARAM unsigned my_n, unsigned my_b, unsigned my_k)
{
inflate_stored_n = my_n;
inflate_stored_b = my_b;
Expand Down Expand Up @@ -1043,7 +1044,15 @@ inflate_unzip_internal(STATE_PARAM transformer_state_t *xstate)
n = (nwrote <0)?nwrote:-1;
goto ret;
}
IF_DESKTOP(n += nwrote;)
#if ENABLE_DESKTOP
long long int v = n + nwrote;
if (v < n) {
bb_simple_error_msg("overflow");
n = -1;
goto ret;
}
n = v;
#endif
if (r == 0) break;
}

Expand All @@ -1054,6 +1063,7 @@ inflate_unzip_internal(STATE_PARAM transformer_state_t *xstate)
bytebuffer_offset--;
bytebuffer[bytebuffer_offset] = gunzip_bb & 0xff;
gunzip_bb >>= 8;
// coverity[overflow_const]
gunzip_bk -= 8;
}
ret:
Expand Down
1 change: 1 addition & 0 deletions src/bled/libbb.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "platform.h"
#include "msapi_utf8.h"

#include <assert.h>
#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
Expand Down
1 change: 1 addition & 0 deletions src/bled/xz_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
/* #define XZ_DEC_ARMTHUMB */
/* #define XZ_DEC_SPARC */

#include <assert.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
Expand Down
1 change: 1 addition & 0 deletions src/bled/xz_dec_bcj.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ static noinline_for_stack size_t XZ_FUNC bcj_x86(
if ((buf[i] & 0xFE) != 0xE8)
continue;

// coverity[overflow_const]
prev_pos = i - prev_pos;
if (prev_pos > 3) {
prev_mask = 0;
Expand Down
7 changes: 5 additions & 2 deletions src/bled/xz_dec_lzma2.c
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ static void XZ_FUNC lzma_len(struct xz_dec_lzma2 *s, struct lzma_len_dec *l,
uint32_t pos_state)
{
uint16_t *probs;
uint32_t limit;
uint32_t limit, v;

if (!rc_bit(&s->rc, &l->choice)) {
probs = l->low[pos_state];
Expand All @@ -641,7 +641,9 @@ static void XZ_FUNC lzma_len(struct xz_dec_lzma2 *s, struct lzma_len_dec *l,
}
}

s->lzma.len += rc_bittree(&s->rc, probs, limit) - limit;
v = s->lzma.len + rc_bittree(&s->rc, probs, limit) - limit;
assert(v >= s->lzma.len);
s->lzma.len = (v < s->lzma.len) ? 0 : v;
}

/* Decode a match. The distance will be stored in s->lzma.rep0. */
Expand All @@ -660,6 +662,7 @@ static void XZ_FUNC lzma_match(struct xz_dec_lzma2 *s, uint32_t pos_state)
lzma_len(s, &s->lzma.match_len_dec, pos_state);

probs = s->lzma.dist_slot[lzma_get_dist_state(s->lzma.len)];
// covertity[overflow_const]
dist_slot = rc_bittree(&s->rc, probs, DIST_SLOTS) - DIST_SLOTS;

if (dist_slot < DIST_MODEL_START) {
Expand Down
18 changes: 12 additions & 6 deletions src/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ BOOL CyclePort(int index)
DWORD size;
USB_CYCLE_PORT_PARAMS cycle_port;

assert(index < MAX_DRIVES);
if_not_assert(index < MAX_DRIVES)
return -1;
// Wait at least 10 secs between resets
if (GetTickCount64() < LastReset + 10000ULL) {
uprintf("You must wait at least 10 seconds before trying to reset a device");
Expand Down Expand Up @@ -190,7 +191,8 @@ int CycleDevice(int index)
SP_DEVINFO_DATA dev_info_data;
SP_PROPCHANGE_PARAMS propchange_params;

assert(index < MAX_DRIVES);
if_not_assert(index < MAX_DRIVES)
return ERROR_INVALID_DRIVE;
if ((index < 0) || (safe_strlen(rufus_drive[index].id) < 8))
return ERROR_INVALID_PARAMETER;

Expand Down Expand Up @@ -583,8 +585,10 @@ BOOL GetDevices(DWORD devnum)

// Better safe than sorry. And yeah, we could have used arrays of
// arrays to avoid this, but it's more readable this way.
assert((uasp_start > 0) && (uasp_start < ARRAYSIZE(usbstor_name)));
assert((card_start > 0) && (card_start < ARRAYSIZE(genstor_name)));
if_not_assert((uasp_start > 0) && (uasp_start < ARRAYSIZE(usbstor_name)))
goto out;
if_not_assert((card_start > 0) && (card_start < ARRAYSIZE(genstor_name)))
goto out;

devid_list = NULL;
if (full_list_size != 0) {
Expand Down Expand Up @@ -671,7 +675,8 @@ BOOL GetDevices(DWORD devnum)
}
// Also test for "_SD&" instead of "_SD_" and so on to allow for devices like
// "SCSI\DiskRicoh_Storage_SD&REV_3.0" to be detected.
assert(strlen(scsi_card_name_copy) > 1);
if_not_assert(strlen(scsi_card_name_copy) > 1)
continue;
scsi_card_name_copy[strlen(scsi_card_name_copy) - 1] = '&';
if (safe_strstr(buffer, scsi_card_name_copy) != NULL) {
props.is_CARD = TRUE;
Expand Down Expand Up @@ -999,7 +1004,8 @@ BOOL GetDevices(DWORD devnum)
rufus_drive[num_drives].display_name = safe_strdup(display_name);
rufus_drive[num_drives].label = safe_strdup(label);
rufus_drive[num_drives].size = drive_size;
assert(rufus_drive[num_drives].size != 0);
if_not_assert(rufus_drive[num_drives].size != 0)
break;
if (hub_path != NULL) {
rufus_drive[num_drives].hub = safe_strdup(hub_path);
rufus_drive[num_drives].port = props.port;
Expand Down
12 changes: 8 additions & 4 deletions src/drive.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,12 @@ char* GetLogicalName(DWORD DriveIndex, uint64_t PartitionOffset, BOOL bKeepTrail

// Sanity checks
len = safe_strlen(volume_name);
assert(len > 4);
assert(safe_strnicmp(volume_name, volume_start, 4) == 0);
assert(volume_name[len - 1] == '\\');
if_not_assert(len > 4)
continue;
if_not_assert(safe_strnicmp(volume_name, volume_start, 4) == 0)
continue;
if_not_assert(volume_name[len - 1] == '\\')
continue;

drive_type = GetDriveTypeA(volume_name);
if ((drive_type != DRIVE_REMOVABLE) && (drive_type != DRIVE_FIXED))
Expand Down Expand Up @@ -1817,7 +1820,8 @@ const char* GetFsName(HANDLE hPhysical, LARGE_INTEGER StartingOffset)
}
}
assert(rev < ARRAYSIZE(ext_names));
ret = ext_names[rev];
if (rev < ARRAYSIZE(ext_names))
ret = ext_names[rev];
goto out;
}

Expand Down
1 change: 1 addition & 0 deletions src/ext2fs/dirhash.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ static void TEA_transform(__u32 buf[4], __u32 const in[])
int n = 16;

do {
// coverity[overflow_const]
sum += DELTA;
b0 += ((b1 << 4)+a) ^ (b1+sum) ^ ((b1 >> 5)+b);
b1 += ((b0 << 4)+c) ^ (b0+sum) ^ ((b0 >> 5)+d);
Expand Down
5 changes: 4 additions & 1 deletion src/ext2fs/initialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
*/

#include "config.h"
#include <assert.h>
#include <stdio.h>
#include <string.h>
#if HAVE_UNISTD_H
Expand Down Expand Up @@ -374,7 +375,9 @@ errcode_t ext2fs_initialize(const char *name, int flags,
* adjust inode count to reflect the adjusted inodes_per_group
*/
if ((__u64)super->s_inodes_per_group * fs->group_desc_count > ~0U) {
ipg--;
assert(ipg != 0);
if (ipg != 0)
ipg--;
goto ipg_retry;
}
super->s_inodes_count = super->s_inodes_per_group *
Expand Down
50 changes: 35 additions & 15 deletions src/format.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ static BOOLEAN __stdcall FormatExCallback(FILE_SYSTEM_CALLBACK_COMMAND Command,
if (IS_ERROR(ErrorStatus))
return FALSE;

assert((actual_fs_type >= 0) && (actual_fs_type < FS_MAX));
if ((actual_fs_type < 0) || (actual_fs_type >= FS_MAX))
if_not_assert((actual_fs_type >= 0) && (actual_fs_type < FS_MAX))
return FALSE;

switch(Command) {
Expand Down Expand Up @@ -1108,6 +1107,10 @@ static int sector_write(int fd, const void* _buf, unsigned int count)

if (sec_size == 0)
sec_size = 512;
if_not_assert(sec_size <= 64 * KB)
return -1;
if_not_assert(count <= 1 * GB)
return -1;

// If we are on a sector boundary and count is multiple of the
// sector size, just issue a regular write
Expand All @@ -1116,6 +1119,8 @@ static int sector_write(int fd, const void* _buf, unsigned int count)

// If we have an existing partial sector, fill and write it
if (sec_buf_pos > 0) {
if_not_assert(sec_size >= sec_buf_pos)
return -1;
fill_size = min(sec_size - sec_buf_pos, count);
memcpy(&sec_buf[sec_buf_pos], buf, fill_size);
sec_buf_pos += fill_size;
Expand All @@ -1133,10 +1138,18 @@ static int sector_write(int fd, const void* _buf, unsigned int count)
written = _write(fd, &buf[fill_size], sec_num * sec_size);
if (written < 0)
return written;
else if (written != sec_num * sec_size)
return fill_size + written;
if (written != sec_num * sec_size) {
// Detect overflows
// coverity[overflow]
int v = fill_size + written;
if_not_assert(v >= fill_size)
return -1;
else
return v;
}
sec_buf_pos = count - fill_size - written;
assert(sec_buf_pos < sec_size);
if_not_assert(sec_buf_pos < sec_size)
return -1;

// Keep leftover bytes, if any, in the sector buffer
if (sec_buf_pos != 0)
Expand Down Expand Up @@ -1180,7 +1193,8 @@ static BOOL WriteDrive(HANDLE hPhysicalDrive, BOOL bZeroDrive)
uprintf("Could not allocate disk zeroing buffer");
goto out;
}
assert((uintptr_t)buffer % SelectedDrive.SectorSize == 0);
if_not_assert((uintptr_t)buffer % SelectedDrive.SectorSize == 0)
goto out;

// Clear buffer
memset(buffer, fast_zeroing ? 0xff : 0x00, buf_size);
Expand All @@ -1192,7 +1206,8 @@ static BOOL WriteDrive(HANDLE hPhysicalDrive, BOOL bZeroDrive)
uprintf("Could not allocate disk comparison buffer");
goto out;
}
assert((uintptr_t)cmp_buffer % SelectedDrive.SectorSize == 0);
if_not_assert((uintptr_t)cmp_buffer % SelectedDrive.SectorSize == 0)
goto out;
}

read_size[0] = buf_size;
Expand Down Expand Up @@ -1290,7 +1305,8 @@ static BOOL WriteDrive(HANDLE hPhysicalDrive, BOOL bZeroDrive)
uprintf("Could not allocate disk write buffer");
goto out;
}
assert((uintptr_t)sec_buf % SelectedDrive.SectorSize == 0);
if_not_assert((uintptr_t)sec_buf% SelectedDrive.SectorSize == 0)
goto out;
sec_buf_pos = 0;
bled_init(256 * KB, uprintf, NULL, sector_write, update_progress, NULL, &ErrorStatus);
bled_ret = bled_uncompress_with_handles(hSourceImage, hPhysicalDrive, img_report.compression_type);
Expand All @@ -1312,7 +1328,8 @@ static BOOL WriteDrive(HANDLE hPhysicalDrive, BOOL bZeroDrive)
goto out;
}
} else {
assert(img_report.compression_type != IMG_COMPRESSION_FFU);
if_not_assert(img_report.compression_type != IMG_COMPRESSION_FFU)
goto out;
// VHD/VHDX require mounting the image first
if (img_report.compression_type == IMG_COMPRESSION_VHD ||
img_report.compression_type == IMG_COMPRESSION_VHDX) {
Expand All @@ -1338,7 +1355,8 @@ static BOOL WriteDrive(HANDLE hPhysicalDrive, BOOL bZeroDrive)
uprintf("Could not allocate disk write buffer");
goto out;
}
assert((uintptr_t)buffer % SelectedDrive.SectorSize == 0);
if_not_assert((uintptr_t)buffer% SelectedDrive.SectorSize == 0)
goto out;

// Start the initial read
ReadFileAsync(hSourceImage, &buffer[read_bufnum * buf_size], (DWORD)MIN(buf_size, target_size));
Expand All @@ -1365,7 +1383,8 @@ static BOOL WriteDrive(HANDLE hPhysicalDrive, BOOL bZeroDrive)

// 2. WriteFile fails unless the size is a multiple of sector size
if (read_size[read_bufnum] % SelectedDrive.SectorSize != 0) {
assert(HI_ALIGN_X_TO_Y(read_size[read_bufnum], SelectedDrive.SectorSize) <= buf_size);
if_not_assert(HI_ALIGN_X_TO_Y(read_size[read_bufnum], SelectedDrive.SectorSize) <= buf_size)
goto out;
read_size[read_bufnum] = HI_ALIGN_X_TO_Y(read_size[read_bufnum], SelectedDrive.SectorSize);
}

Expand Down Expand Up @@ -1660,7 +1679,8 @@ DWORD WINAPI FormatThread(void* param)
if (img_report.compression_type == IMG_COMPRESSION_FFU) {
char cmd[MAX_PATH + 128], *physical = NULL;
// Should have been filtered out beforehand
assert(has_ffu_support);
if_not_assert(has_ffu_support)
goto out;
safe_unlockclose(hPhysicalDrive);
physical = GetPhysicalName(SelectedDrive.DeviceNumber);
static_sprintf(cmd, "dism /Apply-Ffu /ApplyDrive:%s /ImageFile:\"%s\"", physical, image_path);
Expand Down Expand Up @@ -1848,8 +1868,7 @@ DWORD WINAPI FormatThread(void* param)
// All good
} else if (target_type == TT_UEFI) {
// For once, no need to do anything - just check our sanity
assert((boot_type == BT_IMAGE) && IS_EFI_BOOTABLE(img_report) && (fs_type <= FS_NTFS));
if ( (boot_type != BT_IMAGE) || !IS_EFI_BOOTABLE(img_report) || (fs_type > FS_NTFS) ) {
if_not_assert((boot_type == BT_IMAGE) && IS_EFI_BOOTABLE(img_report) && (fs_type <= FS_NTFS)) {
ErrorStatus = RUFUS_ERROR(ERROR_INSTALL_FAILURE);
goto out;
}
Expand Down Expand Up @@ -1924,7 +1943,8 @@ DWORD WINAPI FormatThread(void* param)
ErrorStatus = RUFUS_ERROR(APPERR(ERROR_CANT_PATCH));
}
} else {
assert(!img_report.is_windows_img);
if_not_assert(!img_report.is_windows_img)
goto out;
if (!ExtractISO(image_path, drive_name, FALSE)) {
if (!IS_ERROR(ErrorStatus))
ErrorStatus = RUFUS_ERROR(APPERR(ERROR_ISO_EXTRACT));
Expand Down
Loading

0 comments on commit ceee3e9

Please sign in to comment.