diff --git a/src/runtime_src/tools/xclbinutil/Section.cxx b/src/runtime_src/tools/xclbinutil/Section.cxx index 9c58cb3c439..161c57f08d5 100644 --- a/src/runtime_src/tools/xclbinutil/Section.cxx +++ b/src/runtime_src/tools/xclbinutil/Section.cxx @@ -341,8 +341,8 @@ Section::readXclBinBinary(std::istream& _istream, const axlf_section_header& _se m_name = (char*)&_sectionHeader.m_sectionName; - if (_sectionHeader.m_sectionSize > UINT64_MAX) { - std::string errMsg("FATAL ERROR: Section header size exceeds internal representation size."); + if (_sectionHeader.m_sectionSize > UINT32_MAX) { + std::string errMsg("FATAL ERROR: Section header size exceeds maximum supported section size (4 GiB)."); throw std::runtime_error(errMsg); } @@ -417,8 +417,8 @@ Section::readXclBinBinary(std::istream& _istream, XUtil::TRACE(boost::format("Reading in the section '%s' (%d) as a image.") % getSectionKindAsString() % (unsigned int)getSectionKind()); uint64_t imageSize = XUtil::stringToUInt64(_ptSection.get("Size")); - if (imageSize > UINT64_MAX) { - std::string errMsg("FATAL ERROR: Image size exceeds internal representation size."); + if (imageSize > UINT32_MAX) { + std::string errMsg("FATAL ERROR: Image size exceeds maximum supported section size (4 GiB)."); throw std::runtime_error(errMsg); } diff --git a/src/runtime_src/tools/xclbinutil/SectionAIEPartition.cxx b/src/runtime_src/tools/xclbinutil/SectionAIEPartition.cxx index defdd165c7c..ff2690ffda1 100644 --- a/src/runtime_src/tools/xclbinutil/SectionAIEPartition.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionAIEPartition.cxx @@ -472,6 +472,7 @@ SectionAIEPartition::readSubPayload(const char* pOrigDataSection, static void populate_partition_info(const char* pBase, + size_t bufferSize, const aie_partition_info& aiePartitionInfo, boost::property_tree::ptree& ptAiePartition) { @@ -483,11 +484,18 @@ populate_partition_info(const char* pBase, // Start Columns boost::property_tree::ptree ptStartColumnArray; - const uint16_t* columnArray = reinterpret_cast(pBase + aiePartitionInfo.start_columns.offset); - for (uint32_t index = 0; index < aiePartitionInfo.start_columns.size; index++) { - boost::property_tree::ptree ptElement; - ptElement.put("", (boost::format("%d") % columnArray[index]).str()); - ptStartColumnArray.push_back({ "", ptElement }); + const uint64_t scOffset = aiePartitionInfo.start_columns.offset; + const uint64_t scCount = aiePartitionInfo.start_columns.size; + if (scCount > 0) { + if (scOffset >= bufferSize || scCount > (bufferSize - scOffset) / sizeof(uint16_t)) + throw std::runtime_error("aie_partition_info::start_columns offset/size out of bounds"); + + const uint16_t* columnArray = reinterpret_cast(pBase + scOffset); + for (uint32_t index = 0; index < scCount; index++) { + boost::property_tree::ptree ptElement; + ptElement.put("", (boost::format("%d") % columnArray[index]).str()); + ptStartColumnArray.push_back({ "", ptElement }); + } } ptPartitionInfo.add_child("start_columns", ptStartColumnArray); @@ -497,6 +505,7 @@ populate_partition_info(const char* pBase, // ------------------------------------------------------------------------- static void populate_pre_cdo_groups(const char* pBase, + size_t bufferSize, const cdo_group& aieCDOGroup, boost::property_tree::ptree& ptCDOGroup) { @@ -506,10 +515,15 @@ populate_pre_cdo_groups(const char* pBase, if (aieCDOGroup.pre_cdo_groups.size == 0) return; + const uint64_t offset = aieCDOGroup.pre_cdo_groups.offset; + const uint64_t count = aieCDOGroup.pre_cdo_groups.size; + if (offset >= bufferSize || count > (bufferSize - offset) / sizeof(uint64_t)) + throw std::runtime_error("cdo_group::pre_cdo_groups offset/size out of bounds"); + boost::property_tree::ptree ptPreCDOGroupArray; - const uint64_t* aiePreCDOGroupArray = reinterpret_cast(pBase + aieCDOGroup.pre_cdo_groups.offset); - for (uint32_t index = 0; index < aieCDOGroup.pre_cdo_groups.size; index++) { + const uint64_t* aiePreCDOGroupArray = reinterpret_cast(pBase + offset); + for (uint32_t index = 0; index < count; index++) { const uint64_t& element = aiePreCDOGroupArray[index]; boost::property_tree::ptree ptElement; @@ -524,19 +538,25 @@ populate_pre_cdo_groups(const char* pBase, // ------------------------------------------------------------------------- static void populate_cdo_groups(const char* pBase, + size_t bufferSize, const aie_pdi& aiePDI, boost::property_tree::ptree& ptAiePDI) { XUtil::TRACE("Populating CDO groups"); boost::property_tree::ptree ptCDOGroupArray; - const cdo_group* aieCDOGroupArray = reinterpret_cast(pBase + aiePDI.cdo_groups.offset); - for (uint32_t index = 0; index < aiePDI.cdo_groups.size; index++) { + const uint64_t cdoOffset = aiePDI.cdo_groups.offset; + const uint64_t cdoCount = aiePDI.cdo_groups.size; + if (cdoCount > 0 && (cdoOffset >= bufferSize || cdoCount > (bufferSize - cdoOffset) / sizeof(cdo_group))) + throw std::runtime_error("aie_pdi::cdo_groups offset/size out of bounds"); + + const cdo_group* aieCDOGroupArray = reinterpret_cast(pBase + cdoOffset); + for (uint32_t index = 0; index < cdoCount; index++) { const cdo_group& element = aieCDOGroupArray[index]; boost::property_tree::ptree ptElement; // Name - auto sName = reinterpret_cast(pBase + element.mpo_name); + auto sName = XUtil::bounded_mpo_cstr(pBase, element.mpo_name, bufferSize); ptElement.put("name", sName); XUtil::TRACE("Populating CDO group: " + std::string(sName)); @@ -548,10 +568,15 @@ populate_cdo_groups(const char* pBase, ptElement.put("pdi_id", (boost::format("0x%x") % element.pdi_id).str()); // DPU Kernel IDs - if (element.dpu_kernel_ids.size) { + const uint64_t kidOffset = element.dpu_kernel_ids.offset; + const uint64_t kidCount = element.dpu_kernel_ids.size; + if (kidCount > 0) { + if (kidOffset >= bufferSize || kidCount > (bufferSize - kidOffset) / sizeof(uint64_t)) + throw std::runtime_error("cdo_group::dpu_kernel_ids offset/size out of bounds"); + boost::property_tree::ptree ptDPUKernelIDs; - const uint64_t* kernelIDsArray = reinterpret_cast(pBase + element.dpu_kernel_ids.offset); - for (uint32_t kernelIDindex = 0; kernelIDindex < element.dpu_kernel_ids.size; kernelIDindex++) { + const uint64_t* kernelIDsArray = reinterpret_cast(pBase + kidOffset); + for (uint32_t kernelIDindex = 0; kernelIDindex < kidCount; kernelIDindex++) { boost::property_tree::ptree ptID; ptID.put("", (boost::format("0x%x") % kernelIDsArray[kernelIDindex]).str()); ptDPUKernelIDs.push_back({ "", ptID }); @@ -560,7 +585,7 @@ populate_cdo_groups(const char* pBase, } // Pre cdo groups - populate_pre_cdo_groups(pBase, element, ptElement); + populate_pre_cdo_groups(pBase, bufferSize, element, ptElement); // Add the cdo group element to the array ptCDOGroupArray.push_back({ "", ptElement }); @@ -589,12 +614,14 @@ write_pdi_image(const char* pBase, throw std::runtime_error(errMsg.str()); } + // pdi_image offset/size are validated by the caller before write_pdi_image is invoked. oPDIFile.write(reinterpret_cast(pBase + aiePDI.pdi_image.offset), aiePDI.pdi_image.size); } // ------------------------------------------------------------------------- static void populate_PDIs(const char* pBase, + size_t bufferSize, const fs::path& relativeToDir, const aie_partition& aiePartition, boost::property_tree::ptree& ptAiePartition) @@ -602,21 +629,32 @@ populate_PDIs(const char* pBase, XUtil::TRACE("Populating DPI Array"); boost::property_tree::ptree ptPDIArray; - const aie_pdi* aiePdiArray = reinterpret_cast(pBase + aiePartition.aie_pdi.offset); - for (uint32_t index = 0; index < aiePartition.aie_pdi.size; index++) { + const uint64_t pdiOffset = aiePartition.aie_pdi.offset; + const uint64_t pdiCount = aiePartition.aie_pdi.size; + if (pdiCount > 0 && (pdiOffset >= bufferSize || pdiCount > (bufferSize - pdiOffset) / sizeof(aie_pdi))) + throw std::runtime_error("aie_partition::aie_pdi offset/size out of bounds"); + + const aie_pdi* aiePdiArray = reinterpret_cast(pBase + pdiOffset); + for (uint32_t index = 0; index < pdiCount; index++) { const aie_pdi& element = aiePdiArray[index]; boost::property_tree::ptree ptElement; // UUID ptElement.put("uuid", XUtil::getUUIDAsString(element.uuid)); + // Validate pdi_image before writing + const uint64_t imgOffset = element.pdi_image.offset; + const uint64_t imgSize = element.pdi_image.size; + if (imgSize > 0 && (imgOffset >= bufferSize || imgSize > bufferSize - imgOffset)) + throw std::runtime_error("aie_pdi::pdi_image offset/size out of bounds"); + // Partition Image std::string fileName = XUtil::getUUIDAsString(element.uuid) + ".pdi"; write_pdi_image(pBase, element, fileName, relativeToDir); ptElement.put("file_name", fileName); // CDO Groups - populate_cdo_groups(pBase, element, ptElement); + populate_cdo_groups(pBase, bufferSize, element, ptElement); // Add the PDI element to the array ptPDIArray.push_back({ "", ptElement }); @@ -664,10 +702,10 @@ writeAIEPartitionImage(const char* pBuffer, ptAiePartition.put("kernel_commit_id", sKernelCommitId); // Partition info - populate_partition_info(pBuffer, pHdr->info, ptAiePartition); + populate_partition_info(pBuffer, bufferSize, pHdr->info, ptAiePartition); // PDIs - populate_PDIs(pBuffer, relativeToDir, *pHdr, ptAiePartition); + populate_PDIs(pBuffer, bufferSize, relativeToDir, *pHdr, ptAiePartition); // Write out the built property tree boost::property_tree::ptree ptRoot; diff --git a/src/runtime_src/tools/xclbinutil/SectionClockFrequencyTopology.cxx b/src/runtime_src/tools/xclbinutil/SectionClockFrequencyTopology.cxx index 3adac82abc0..24461d9ccea 100644 --- a/src/runtime_src/tools/xclbinutil/SectionClockFrequencyTopology.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionClockFrequencyTopology.cxx @@ -125,14 +125,14 @@ SectionClockFrequencyTopology::marshalToJSON(char* _pDataSection, % index % (unsigned int)pHdr->m_clock_freq[index].m_freq_Mhz % getClockTypeStr((CLOCK_TYPE)pHdr->m_clock_freq[index].m_type) - % pHdr->m_clock_freq[index].m_name); + % XUtil::bounded_fixed_cstr(pHdr->m_clock_freq[index].m_name)); // Write out the entire structure XUtil::TRACE_BUF("clock_freq", reinterpret_cast(&pHdr->m_clock_freq[index]), sizeof(clock_freq)); clock_freq.put("m_freq_Mhz", (boost::format("%d") % (unsigned int)pHdr->m_clock_freq[index].m_freq_Mhz).str()); clock_freq.put("m_type", getClockTypeStr((CLOCK_TYPE)pHdr->m_clock_freq[index].m_type).c_str()); - clock_freq.put("m_name", (boost::format("%s") % pHdr->m_clock_freq[index].m_name).str()); + clock_freq.put("m_name", XUtil::bounded_fixed_cstr(pHdr->m_clock_freq[index].m_name)); m_clock_freq.push_back({ "", clock_freq }); // Used to make an array of objects } diff --git a/src/runtime_src/tools/xclbinutil/SectionDNACertificate.cxx b/src/runtime_src/tools/xclbinutil/SectionDNACertificate.cxx index c2231429c6c..aba9ee0835b 100644 --- a/src/runtime_src/tools/xclbinutil/SectionDNACertificate.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionDNACertificate.cxx @@ -92,8 +92,8 @@ SectionDNACertificate::marshalToJSON(char* _pDataSection, throw std::runtime_error(errMsg.str()); } - if (((dnaEntriesBitSize / 8) > _sectionSize)) { - auto errMsg = boost::format("ERROR: The message DNA length (0x%x bytes) exceeds the DNA_CERTIFICATE size (0x%x bytes).") % (dnaEntriesBitSize / 8) % _sectionSize; + if (((dnaEntriesBitSize / 8) + signatureSizeBytes + sizeof(uint64_t)) > _sectionSize) { + auto errMsg = boost::format("ERROR: The message DNA length (0x%x bytes) plus overhead exceeds the DNA_CERTIFICATE size (0x%x bytes).") % (dnaEntriesBitSize / 8) % _sectionSize; throw std::runtime_error(errMsg.str()); } @@ -103,7 +103,12 @@ SectionDNACertificate::marshalToJSON(char* _pDataSection, // Get padding string std::string sPadding; uint64_t paddingOffset = dnaEntryCount * dnaEntrySizeBytes; - uint64_t paddingSize = (_sectionSize - signatureSizeBytes) - paddingOffset; + uint64_t usableSize = _sectionSize - signatureSizeBytes; + if (paddingOffset > usableSize) { + auto errMsg = boost::format("ERROR: DNA entries (0x%lx bytes) exceed usable section space (0x%lx bytes).") % paddingOffset % usableSize; + throw std::runtime_error(errMsg.str()); + } + uint64_t paddingSize = usableSize - paddingOffset; XUtil::binaryBufferToHexString((unsigned char*)&_pDataSection[paddingOffset], paddingSize, sPadding); diff --git a/src/runtime_src/tools/xclbinutil/SectionDebugIPLayout.cxx b/src/runtime_src/tools/xclbinutil/SectionDebugIPLayout.cxx index b1898e8e536..878bc9c0620 100644 --- a/src/runtime_src/tools/xclbinutil/SectionDebugIPLayout.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionDebugIPLayout.cxx @@ -198,7 +198,7 @@ SectionDebugIPLayout::marshalToJSON(char* _pDataSection, % static_cast(pHdr->m_debug_ip_data[index].m_major) % static_cast(pHdr->m_debug_ip_data[index].m_minor) % pHdr->m_debug_ip_data[index].m_base_address - % pHdr->m_debug_ip_data[index].m_name); + % XUtil::bounded_fixed_cstr(pHdr->m_debug_ip_data[index].m_name)); // Write out the entire structure XUtil::TRACE_BUF("debug_ip_data", reinterpret_cast(&pHdr->m_debug_ip_data[index]), sizeof(debug_ip_data)); @@ -209,7 +209,7 @@ SectionDebugIPLayout::marshalToJSON(char* _pDataSection, debug_ip_data.put("m_major", (boost::format("%d") % static_cast(pHdr->m_debug_ip_data[index].m_major)).str()); debug_ip_data.put("m_minor", (boost::format("%d") % static_cast(pHdr->m_debug_ip_data[index].m_minor)).str()); debug_ip_data.put("m_base_address", (boost::format("0x%lx") % pHdr->m_debug_ip_data[index].m_base_address).str()); - debug_ip_data.put("m_name", (boost::format("%s") % pHdr->m_debug_ip_data[index].m_name).str()); + debug_ip_data.put("m_name", XUtil::bounded_fixed_cstr(pHdr->m_debug_ip_data[index].m_name)); m_debug_ip_data.push_back({ "", debug_ip_data }); // Used to make an array of objects } diff --git a/src/runtime_src/tools/xclbinutil/SectionGroupTopology.cxx b/src/runtime_src/tools/xclbinutil/SectionGroupTopology.cxx index e817b871205..4a6080d719d 100644 --- a/src/runtime_src/tools/xclbinutil/SectionGroupTopology.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionGroupTopology.cxx @@ -160,7 +160,7 @@ SectionGroupTopology::marshalToJSON(char* _pDataSection, % getMemTypeStr((MEM_TYPE)pHdr->m_mem_data[index].m_type) % (unsigned int)pHdr->m_mem_data[index].m_used % pHdr->m_mem_data[index].m_size - % pHdr->m_mem_data[index].m_tag + % XUtil::bounded_fixed_cstr(pHdr->m_mem_data[index].m_tag) % pHdr->m_mem_data[index].m_base_address); // Write out the entire structure @@ -169,7 +169,7 @@ SectionGroupTopology::marshalToJSON(char* _pDataSection, mem_data.put("m_type", getMemTypeStr((MEM_TYPE)pHdr->m_mem_data[index].m_type).c_str()); mem_data.put("m_used", (boost::format("%d") % (unsigned int)pHdr->m_mem_data[index].m_used).str()); mem_data.put("m_sizeKB", (boost::format("0x%lx") % pHdr->m_mem_data[index].m_size).str()); - mem_data.put("m_tag", (boost::format("%s") % pHdr->m_mem_data[index].m_tag).str()); + mem_data.put("m_tag", XUtil::bounded_fixed_cstr(pHdr->m_mem_data[index].m_tag)); mem_data.put("m_base_address", (boost::format("0x%lx") % pHdr->m_mem_data[index].m_base_address).str()); m_mem_data.push_back({ "", mem_data }); // Used to make an array of objects diff --git a/src/runtime_src/tools/xclbinutil/SectionIPLayout.cxx b/src/runtime_src/tools/xclbinutil/SectionIPLayout.cxx index ecd2e51ffef..2570184c9cf 100644 --- a/src/runtime_src/tools/xclbinutil/SectionIPLayout.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionIPLayout.cxx @@ -262,7 +262,7 @@ SectionIPLayout::marshalToJSON(char* _pDataSection, % pHdr->m_ip_data[index].indices.m_index % pHdr->m_ip_data[index].indices.m_pc_index % pHdr->m_ip_data[index].m_base_address - % pHdr->m_ip_data[index].m_name); + % XUtil::bounded_fixed_cstr(pHdr->m_ip_data[index].m_name)); } else if ((IP_TYPE)pHdr->m_ip_data[index].m_type == IP_KERNEL) { std::string sIPControlType = getIPControlTypeStr((IP_CONTROL)((pHdr->m_ip_data[index].properties & ((uint32_t)IP_CONTROL_MASK)) >> IP_CONTROL_SHIFT)); XUtil::TRACE(boost::format("[%d]: m_type: %s, properties: 0x%x {m_ip_control: %s, m_interrupt_id: %d, m_int_enable: %d}, m_base_address: 0x%lx, m_name: '%s'") @@ -273,7 +273,7 @@ SectionIPLayout::marshalToJSON(char* _pDataSection, % ((pHdr->m_ip_data[index].properties & ((uint32_t)IP_INTERRUPT_ID_MASK)) >> IP_INTERRUPT_ID_SHIFT) % (pHdr->m_ip_data[index].properties & ((uint32_t)IP_INT_ENABLE_MASK)) % pHdr->m_ip_data[index].m_base_address - % pHdr->m_ip_data[index].m_name); + % XUtil::bounded_fixed_cstr(pHdr->m_ip_data[index].m_name)); } else { // IP_PS_KERNEL // if m_subtype is ST_DPU (i.e. fixed ps kernel), display "m_subtype", "m_functional" and "m_kernel_id" @@ -286,14 +286,14 @@ SectionIPLayout::marshalToJSON(char* _pDataSection, % getFunctionalStr((PS_FUNCTIONAL)pHdr->m_ip_data[index].ps_kernel.m_functional) % (unsigned int)pHdr->m_ip_data[index].ps_kernel.m_kernel_id % pHdr->m_ip_data[index].m_base_address - % pHdr->m_ip_data[index].m_name); + % XUtil::bounded_fixed_cstr(pHdr->m_ip_data[index].m_name)); } else { XUtil::TRACE(boost::format("[%d]: m_type: %s, properties: 0x%x, m_base_address: 0x%lx, m_name: '%s'") % index % getIPTypeStr((IP_TYPE)pHdr->m_ip_data[index].m_type) % pHdr->m_ip_data[index].properties % pHdr->m_ip_data[index].m_base_address - % pHdr->m_ip_data[index].m_name); + % XUtil::bounded_fixed_cstr(pHdr->m_ip_data[index].m_name)); } } @@ -341,7 +341,7 @@ SectionIPLayout::marshalToJSON(char* _pDataSection, } else { ptIPEntry.put("m_base_address", "not_used"); } - ptIPEntry.put("m_name", (boost::format("%s") % pHdr->m_ip_data[index].m_name).str()); + ptIPEntry.put("m_name", XUtil::bounded_fixed_cstr(pHdr->m_ip_data[index].m_name)); ptIPData.push_back({ "", ptIPEntry }); // Used to make an array of objects } diff --git a/src/runtime_src/tools/xclbinutil/SectionMCS.cxx b/src/runtime_src/tools/xclbinutil/SectionMCS.cxx index b38c50ebcd4..5fb07acfddf 100644 --- a/src/runtime_src/tools/xclbinutil/SectionMCS.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionMCS.cxx @@ -242,21 +242,21 @@ SectionMCS::extractBuffers(const char* _pDataSection, XUtil::TRACE_BUF("m_chunk", reinterpret_cast(&(pHdr->m_chunk[index])), sizeof(mcs_chunk)); - const char* ptrImageBase = _pDataSection + pHdr->m_chunk[index].m_offset; - - // Check to make sure that the MCS image is partially looking good - if ((uint64_t)ptrImageBase > ((uint64_t)_pDataSection) + _sectionSize) { - auto errMsg = boost::format("ERROR: MCS image %d start offset exceeds MCS segment size.") % index; + // Validate offset and size directly in offset-space to avoid pointer wraparound (CWE-190) + const uint64_t chunkOffset = pHdr->m_chunk[index].m_offset; + const uint64_t chunkSize = pHdr->m_chunk[index].m_size; + if (chunkOffset >= _sectionSize) { + auto errMsg = boost::format("ERROR: MCS image %d start offset (0x%lx) exceeds MCS segment size (0x%lx).") % index % chunkOffset % _sectionSize; throw std::runtime_error(errMsg.str()); } - - if (((uint64_t)ptrImageBase) + pHdr->m_chunk[index].m_size > ((uint64_t)_pDataSection) + _sectionSize) { - auto errMsg = boost::format("ERROR: MCS image %d size exceeds the MCS segment size.") % index; + if (chunkSize > _sectionSize - chunkOffset) { + auto errMsg = boost::format("ERROR: MCS image %d size (0x%lx) exceeds the MCS segment size.") % index % chunkSize; throw std::runtime_error(errMsg.str()); } + const char* ptrImageBase = _pDataSection + chunkOffset; std::ostringstream* pBuffer = new std::ostringstream; - pBuffer->write(ptrImageBase, pHdr->m_chunk[index].m_size); + pBuffer->write(ptrImageBase, chunkSize); _mcsBuffers.emplace_back((MCS_TYPE)pHdr->m_chunk[index].m_type, pBuffer); } diff --git a/src/runtime_src/tools/xclbinutil/SectionMemTopology.cxx b/src/runtime_src/tools/xclbinutil/SectionMemTopology.cxx index 4d56a221a78..ee0aeb6d98b 100644 --- a/src/runtime_src/tools/xclbinutil/SectionMemTopology.cxx +++ b/src/runtime_src/tools/xclbinutil/SectionMemTopology.cxx @@ -159,7 +159,7 @@ SectionMemTopology::marshalToJSON(char* _pDataSection, % getMemTypeStr((MEM_TYPE)pHdr->m_mem_data[index].m_type) % (unsigned int)pHdr->m_mem_data[index].m_used % pHdr->m_mem_data[index].m_size - % pHdr->m_mem_data[index].m_tag + % XUtil::bounded_fixed_cstr(pHdr->m_mem_data[index].m_tag) % pHdr->m_mem_data[index].m_base_address); // Write out the entire structure @@ -168,7 +168,7 @@ SectionMemTopology::marshalToJSON(char* _pDataSection, mem_data.put("m_type", getMemTypeStr((MEM_TYPE)pHdr->m_mem_data[index].m_type).c_str()); mem_data.put("m_used", (boost::format("%d") % (unsigned int)pHdr->m_mem_data[index].m_used).str()); mem_data.put("m_sizeKB", (boost::format("0x%lx") % pHdr->m_mem_data[index].m_size).str()); - mem_data.put("m_tag", (boost::format("%s") % pHdr->m_mem_data[index].m_tag).str()); + mem_data.put("m_tag", XUtil::bounded_fixed_cstr(pHdr->m_mem_data[index].m_tag)); mem_data.put("m_base_address", (boost::format("0x%lx") % pHdr->m_mem_data[index].m_base_address).str()); m_mem_data.push_back({ "", mem_data }); // Used to make an array of objects diff --git a/src/runtime_src/tools/xclbinutil/XclBinClass.cxx b/src/runtime_src/tools/xclbinutil/XclBinClass.cxx index c4b8e9cb08e..487d08dfd5d 100644 --- a/src/runtime_src/tools/xclbinutil/XclBinClass.cxx +++ b/src/runtime_src/tools/xclbinutil/XclBinClass.cxx @@ -1969,6 +1969,9 @@ XclBin::updateInterfaceuuid() } // Updating axlf header interface_uuid with interface_uuid from partition_metadata + if (ptInterfaces.empty()) + return; + boost::property_tree::ptree ptInterface = ptInterfaces[0]; auto sInterfaceUUID = ptInterface.get("interface_uuid", "00000000-0000-0000-0000-000000000000"); sInterfaceUUID.erase(std::remove(sInterfaceUUID.begin(), sInterfaceUUID.end(), '-'), sInterfaceUUID.end()); // Remove the '-' diff --git a/src/runtime_src/tools/xclbinutil/XclBinUtilities.cxx b/src/runtime_src/tools/xclbinutil/XclBinUtilities.cxx index c90033429a7..4cc6b08d393 100644 --- a/src/runtime_src/tools/xclbinutil/XclBinUtilities.cxx +++ b/src/runtime_src/tools/xclbinutil/XclBinUtilities.cxx @@ -430,22 +430,30 @@ XclBinUtilities::getSignature(std::fstream& _istream, std::string& _sSignature, _istream.seekg(signatureOffset); _istream.read((char*)&signature, sizeof(XUtil::SignatureHeader)); + if (_istream.gcount() != static_cast(sizeof(XUtil::SignatureHeader))) + throw std::runtime_error("ERROR: Short read of signature header"); - // Get signedBy + // Get signedBy — compute seek position as uint64_t to avoid unsigned int overflow (CWE-190) if (signature.signedBySize != 0) { - _istream.seekg(signatureOffset + signature.signedByOffset); + _istream.seekg(static_cast(signatureOffset) + signature.signedByOffset); std::unique_ptr data( new char[ signature.signedBySize ] ); _istream.read( data.get(), signature.signedBySize ); + if (_istream.gcount() != static_cast(signature.signedBySize)) + throw std::runtime_error("ERROR: Short read of signedBy field in signature"); + _sSignedBy = std::string(data.get(), signature.signedBySize); } // Get the signature if (signature.signatureSize != 0) { - _istream.seekg(signatureOffset + signature.signatureOffset); + _istream.seekg(static_cast(signatureOffset) + signature.signatureOffset); std::unique_ptr data( new char[ signature.signatureSize ] ); _istream.read( data.get(), signature.signatureSize ); + if (_istream.gcount() != static_cast(signature.signatureSize)) + throw std::runtime_error("ERROR: Short read of signature field"); + _sSignature = std::string(data.get(), signature.signatureSize); } @@ -777,7 +785,7 @@ createMemoryBankGroupEntries( std::vector & workingConnection for (unsigned int idx = 0; idx < memIndexVector.size();) { auto s_index = idx; - while ((memIndexVector[idx] + 1) == memIndexVector[idx + 1]) + while ((idx + 1 < memIndexVector.size()) && ((memIndexVector[idx] + 1) == memIndexVector[idx + 1])) idx++; newTag += std::to_string(memIndexVector[s_index]); @@ -1243,6 +1251,22 @@ XclBinUtilities::exec(const fs::path &cmd, } #else +// Shell-quote a single argument by wrapping in single quotes and escaping any +// embedded single quotes as '"'"' (end-quote, literal-quote, re-open-quote). +static std::string +shell_quote(const std::string& s) +{ + std::string result = "'"; + for (char c : s) { + if (c == '\'') + result += "'\"'\"'"; + else + result += c; + } + result += "'"; + return result; +} + int XclBinUtilities::exec(const fs::path &cmd, const std::vector &args, @@ -1250,8 +1274,11 @@ XclBinUtilities::exec(const fs::path &cmd, std::ostringstream & os_stdout, std::ostringstream & os_stderr) { - // Build the command line - const std::string cmdLine = cmd.string() + " " + boost::algorithm::join(args, " "); + // Build the command line with each argument individually shell-quoted + // to prevent command injection via user-controlled paths (CWE-78). + std::string cmdLine = shell_quote(cmd.string()); + for (const auto& arg : args) + cmdLine += " " + shell_quote(arg); std::array buffer; std::string result; diff --git a/src/runtime_src/tools/xclbinutil/XclBinUtilities.h b/src/runtime_src/tools/xclbinutil/XclBinUtilities.h index bdd3312b176..ca4bfcaf797 100644 --- a/src/runtime_src/tools/xclbinutil/XclBinUtilities.h +++ b/src/runtime_src/tools/xclbinutil/XclBinUtilities.h @@ -144,6 +144,24 @@ void safeStringCopy(char* _destBuffer, const std::string& _source, unsigned int // Validates that the offset lies within the buffer and that a null terminator exists // before the buffer end. Throws std::runtime_error on violation (SWSPLAT-30717/CWE-125). const char* bounded_mpo_cstr(const void* pHdr, uint32_t mpo_offset, size_t bufferSize); + +// bounded_fixed_cstr - Safely convert a fixed-size embedded char/uint8_t array (CWE-125). +// Wire-format structs embed fixed-size arrays that may not be null-terminated when an +// attacker controls the xclbin. Returns a std::string of at most N chars, stopping at NUL. +template +std::string +bounded_fixed_cstr(const char (&field)[N]) +{ + return std::string(field, strnlen(field, N)); +} + +template +std::string +bounded_fixed_cstr(const unsigned char (&field)[N]) +{ + return std::string(reinterpret_cast(field), strnlen(reinterpret_cast(field), N)); +} + unsigned int bytesToAlign(uint64_t _offset); unsigned int alignBytes(std::ostream & _buf, unsigned int _byteBoundary);