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 7a23298
Show file tree
Hide file tree
Showing 24 changed files with 120 additions and 67 deletions.
5 changes: 5 additions & 0 deletions 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 ((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
1 change: 1 addition & 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
15 changes: 12 additions & 3 deletions src/bled/decompress_gunzip.c
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,15 @@ static unsigned fill_bitbuffer(STATE_PARAM unsigned bitbuffer, unsigned *current
bytebuffer_size += 4;
bytebuffer_offset = 4;
}
bitbuffer |= ((unsigned) bytebuffer[bytebuffer_offset]) << *current;

uint64_t bbf = (uint64_t)bytebuffer[bytebuffer_offset];
bbf <<= *current;
assert(bbf < UINT32_MAX);
if (bbf >= UINT32_MAX) {
error_msg = "overflow";
abort_unzip(PASS_STATE_ONLY);
}
bitbuffer |= (unsigned) bbf;
bytebuffer_offset++;
*current += 8;
}
Expand Down Expand Up @@ -661,7 +669,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 @@ -1038,7 +1046,8 @@ inflate_unzip_internal(STATE_PARAM transformer_state_t *xstate)
while (1) {
int r = inflate_get_next_window(PASS_STATE_ONLY);
nwrote = transformer_write(xstate, gunzip_window, gunzip_outbuf_count);
if (nwrote != (ssize_t)gunzip_outbuf_count) {
// Coverity is dumb...
if (nwrote < 0 || nwrote != (ssize_t)gunzip_outbuf_count || nwrote > INT32_MAX) {
huft_free_all(PASS_STATE_ONLY);
n = (nwrote <0)?nwrote:-1;
goto 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
12 changes: 10 additions & 2 deletions src/bled/xz_dec_lzma2.c
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,11 @@ 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;
assert(rc_bittree(&s->rc, probs, limit) >= limit);
if (rc_bittree(&s->rc, probs, limit) >= limit)
s->lzma.len += rc_bittree(&s->rc, probs, limit) - limit;
else
s->lzma.len = 0;
}

/* Decode a match. The distance will be stored in s->lzma.rep0. */
Expand All @@ -660,7 +664,11 @@ 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)];
dist_slot = rc_bittree(&s->rc, probs, DIST_SLOTS) - DIST_SLOTS;
assert(rc_bittree(&s->rc, probs, DIST_SLOTS) >= DIST_SLOTS);
if (rc_bittree(&s->rc, probs, DIST_SLOTS) >= DIST_SLOTS)
dist_slot = rc_bittree(&s->rc, probs, DIST_SLOTS) - DIST_SLOTS;
else
dist_slot = 0;

if (dist_slot < DIST_MODEL_START) {
s->lzma.rep0 = dist_slot;
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
39 changes: 26 additions & 13 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 @@ -1136,7 +1141,8 @@ static int sector_write(int fd, const void* _buf, unsigned int count)
else if (written != sec_num * sec_size)
return fill_size + written;
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 +1186,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 +1199,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 +1298,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 +1321,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 +1348,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 +1376,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 +1672,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 +1861,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 +1936,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
8 changes: 4 additions & 4 deletions src/iso.c
Original file line number Diff line number Diff line change
Expand Up @@ -1571,8 +1571,8 @@ uint32_t ReadISOFileToBuffer(const char* iso, const char* iso_file, uint8_t** bu
goto out;
}
file_length = udf_get_file_length(p_udf_file);
if (file_length > UINT32_MAX) {
uprintf("Only files smaller than 4 GB are supported");
if (file_length > 1 * GB) {
uprintf("Only files smaller than 1 GB are supported");
goto out;
}
nblocks = (uint32_t)((file_length + UDF_BLOCKSIZE - 1) / UDF_BLOCKSIZE);
Expand Down Expand Up @@ -1604,8 +1604,8 @@ uint32_t ReadISOFileToBuffer(const char* iso, const char* iso_file, uint8_t** bu
goto out;
}
file_length = p_statbuf->total_size;
if (file_length > UINT32_MAX) {
uprintf("Only files smaller than 4 GB are supported");
if (file_length > 1 * GB) {
uprintf("Only files smaller than 1 GB are supported");
goto out;
}
nblocks = (uint32_t)((file_length + ISO_BLOCKSIZE - 1) / ISO_BLOCKSIZE);
Expand Down
6 changes: 2 additions & 4 deletions src/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,7 @@ static DWORD WINAPI SearchProcessThread(LPVOID param)
// Work on our own copy of the handle names so we don't have to hold the
// mutex for string comparison. Update only if the version has changed.
if (blocking_process.nVersion[0] != blocking_process.nVersion[1]) {
assert(blocking_process.wHandleName != NULL && blocking_process.nHandles != 0);
if (blocking_process.wHandleName == NULL || blocking_process.nHandles == 0) {
if_not_assert(blocking_process.wHandleName != NULL && blocking_process.nHandles != 0) {
ReleaseMutex(hLock);
goto out;
}
Expand Down Expand Up @@ -930,8 +929,7 @@ BYTE GetProcessSearch(uint32_t timeout, uint8_t access_mask, BOOL bIgnoreStalePr
return 0;
}

assert(blocking_process.hLock != NULL);
if (blocking_process.hLock == NULL)
if_not_assert(blocking_process.hLock != NULL)
return 0;

retry:
Expand Down
Loading

0 comments on commit 7a23298

Please sign in to comment.