From 4c5c0151aca3d3917386702a512e624fa49f63b3 Mon Sep 17 00:00:00 2001 From: Henry Gabryjelski Date: Mon, 30 Dec 2024 19:14:37 -0800 Subject: [PATCH] CodeQL fixes Fixes various errors automaticallly detected by CodeQL. * Too few arguments to formatting function * Wrong type of arguments to formatting function * `size_t` uses format specifier `z` * `uint32_t` uses format specifier `PRIx32` Also fix various build warnings. * Fix: comparison of integer expressions of different signedness [-Wsign-compare] * Fix: operation on 'i' may be undefined [-Wsequence-point] * Fix: 'expected_redundancy' may be used uninitialized [-Wmaybe-uninitialized] --- bintool/bintool.cpp | 6 +++--- bintool/metadata.h | 5 +++-- elf/elf_file.cpp | 2 +- elf2uf2/elf2uf2.cpp | 3 ++- main.cpp | 20 +++++++++++++++----- otp_header_parser/otp_header_parse.cpp | 2 +- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/bintool/bintool.cpp b/bintool/bintool.cpp index c8fa8dc..81be9ca 100644 --- a/bintool/bintool.cpp +++ b/bintool/bintool.cpp @@ -4,7 +4,7 @@ #include #include #include - +#include #include #include "boot/picobin.h" @@ -646,7 +646,7 @@ std::vector get_lm_hash_data(elf_file *elf, block *new_block, bool clea const auto data = elf->content(*seg); // std::cout << "virt = " << std::hex << seg->virtual_address() << " + " << std::hex << seg->virtual_size() << ", phys = " << std::hex << seg->physical_address() << " + " << std::hex << seg->physical_size() << std::endl; if (data.size() != seg->physical_size()) { - fail(ERROR_INCOMPATIBLE, "Elf segment physical size (%x) does not match data size in file (%x)", seg->physical_size(), data.size()); + fail(ERROR_INCOMPATIBLE, "Elf segment physical size (%" PRIx32 ") does not match data size in file (%zx)", seg->physical_size(), data.size()); } if (seg->physical_size() && seg->physical_address() < new_block->physical_addr) { std::copy(data.begin(), data.end(), std::back_inserter(to_hash)); @@ -899,7 +899,7 @@ int encrypt(elf_file *elf, block *new_block, const private_t aes_key, const publ std::vector data(enc_data.begin() + i, enc_data.begin() + i + seg->physical_size()); // std::cout << "virt = " << std::hex << seg->virtual_address() << " + " << std::hex << seg->virtual_size() << ", phys = " << std::hex << seg->physical_address() << " + " << std::hex << seg->physical_size() << std::endl; if (data.size() != seg->physical_size()) { - fail(ERROR_INCOMPATIBLE, "Elf segment physical size (%x) does not match data size in file (%x)", seg->physical_size(), data.size()); + fail(ERROR_INCOMPATIBLE, "Elf segment physical size (%" PRIx32 ") does not match data size in file (%zx)", seg->physical_size(), data.size()); } if (seg->physical_size() && seg->physical_address() < new_block->physical_addr) { DEBUG_LOG("ENCRYPTED %08x + %08x\n", (int)seg->physical_address(), (int)seg->physical_size()); diff --git a/bintool/metadata.h b/bintool/metadata.h index 605bf63..fd0359b 100644 --- a/bintool/metadata.h +++ b/bintool/metadata.h @@ -238,7 +238,7 @@ struct partition_table_item : public single_byte_size_item { for (unsigned int i=2; i < size; i++) { data.push_back(*it++); } - int i=0; + size_t i=0; while (i < data.size()) { partition new_p; uint32_t permissions_locations = data[i++]; @@ -255,7 +255,8 @@ struct partition_table_item : public single_byte_size_item { new_p.flags = permissions_flags & (~PICOBIN_PARTITION_PERMISSIONS_BITS); if (new_p.flags & PICOBIN_PARTITION_FLAGS_HAS_ID_BITS) { - new_p.id = (uint64_t)data[i++] | ((uint64_t)data[i++] << 32); + new_p.id = (uint64_t)data[i] | ((uint64_t)data[i+1] << 32); + i += 2; } uint8_t num_extra_families = (new_p.flags & PICOBIN_PARTITION_FLAGS_ACCEPTS_NUM_EXTRA_FAMILIES_BITS) >> PICOBIN_PARTITION_FLAGS_ACCEPTS_NUM_EXTRA_FAMILIES_LSB; diff --git a/elf/elf_file.cpp b/elf/elf_file.cpp index febb318..18e20d9 100644 --- a/elf/elf_file.cpp +++ b/elf/elf_file.cpp @@ -190,7 +190,7 @@ int rp_determine_binary_type(const elf32_header &eh, const std::vector elf_bytes.size()) { - fail(ERROR_FORMAT, "ELF File Read from 0x%x with size 0x%x exceeds the file size 0x%x", offset, length, elf_bytes.size()); + fail(ERROR_FORMAT, "ELF File Read from 0x%x with size 0x%x exceeds the file size 0x%zx", offset, length, elf_bytes.size()); } memcpy(dest, &elf_bytes[offset], length); } diff --git a/elf2uf2/elf2uf2.cpp b/elf2uf2/elf2uf2.cpp index 63ce372..91f30be 100644 --- a/elf2uf2/elf2uf2.cpp +++ b/elf2uf2/elf2uf2.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include "elf2uf2.h" #include "errors.h" @@ -40,7 +41,7 @@ int check_address_range(const address_ranges& valid_ranges, uint32_t addr, uint3 for(const auto& range : valid_ranges) { if (range.from <= addr && range.to >= addr + size) { if (range.type == address_range::type::NO_CONTENTS && !uninitialized) { - fail(ERROR_INCOMPATIBLE, "ELF contains memory contents for uninitialized memory at %p", addr); + fail(ERROR_INCOMPATIBLE, "ELF contains memory contents for uninitialized memory at 0x%08" PRIx32, addr); } ar = range; if (verbose) { diff --git a/main.cpp b/main.cpp index 1f75cd3..a739dd8 100644 --- a/main.cpp +++ b/main.cpp @@ -3050,7 +3050,7 @@ void info_guts(memory_access &raw_access, void *con) { unpartitioned << ", uf2 { " << cli::join(family_ids, ", ") << " }"; info_pair("un-partitioned space", unpartitioned.str()); - for (int i=0; i < partition_table->partitions.size(); i++) { + for (size_t i=0; i < partition_table->partitions.size(); i++) { std::stringstream pstring; std::stringstream pname; auto partition = partition_table->partitions[i]; @@ -4363,8 +4363,18 @@ bool erase_command::execute(device_map &devices) { if (settings.load.partition >= partitions->size()) { fail(ERROR_NOT_POSSIBLE, "There are only %d partitions on the device", partitions->size()); } - start = std::get<0>((*partitions)[settings.load.partition]); - end = std::get<1>((*partitions)[settings.load.partition]); + size_t tmp; + tmp = std::get<0>((*partitions)[settings.load.partition]); + if (tmp > UINT32_MAX) { + fail(ERROR_NOT_POSSIBLE, "Partition start address is too large"); + } + start = tmp; + tmp = std::get<1>((*partitions)[settings.load.partition]); + if (tmp > UINT32_MAX) { + fail(ERROR_NOT_POSSIBLE, "Partition end address is too large"); + } + end = tmp; + printf("Erasing partition %d:\n", settings.load.partition); printf(" %08x->%08x\n", start, end); start += FLASH_START; @@ -5415,7 +5425,7 @@ bool get_mask(const std::string& sel, uint32_t &mask, int max_bit) { bool ok = get_int(sel.substr(0, dash), from) && get_int(sel.substr(dash+1), to); if (!ok || to < from || from < 0 || to >= max_bit) { - fail(ERROR_ARGS, "Invalid bit-range in selector: %s; expect 'm-n' where m >= 0, n >= m and n <= %d", max_bit-1); + fail(ERROR_ARGS, "Invalid bit-range in selector: %s; expect 'm-n' where m >= 0, n >= m and n <= %d", sel.c_str(), max_bit-1); } mask = (2u << to) - (1u << from); return true; @@ -5991,7 +6001,7 @@ bool partition_create_command::execute(device_map &devices) { cur_pos = start + size; #if SUPPORT_A2 if (start <= (settings.uf2.abs_block_loc - FLASH_START)/0x1000 && start + size > (settings.uf2.abs_block_loc - FLASH_START)/0x1000) { - fail(ERROR_INCOMPATIBLE, "The address %lx cannot be in a partition for the RP2350-E10 fix to work", settings.uf2.abs_block_loc); + fail(ERROR_INCOMPATIBLE, "The address %" PRIx32 " cannot be in a partition for the RP2350-E10 fix to work", settings.uf2.abs_block_loc); } #endif new_p.first_sector = start; diff --git a/otp_header_parser/otp_header_parse.cpp b/otp_header_parser/otp_header_parse.cpp index 00e924c..b6ec2d0 100644 --- a/otp_header_parser/otp_header_parse.cpp +++ b/otp_header_parser/otp_header_parse.cpp @@ -147,7 +147,7 @@ int main(int argc, char **argv) { std::string reg_name; std::string field_name; std::string comment; - int expected_redundancy; + int expected_redundancy = 1; otp_reg reg; otp_field field; while (std::getline(infile, line)) {