Security review of xclbinutil

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<std::string>("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);
     }
 

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<const uint16_t*>(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<const uint16_t*>(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<const uint64_t*>(pBase + aieCDOGroup.pre_cdo_groups.offset);
-  for (uint32_t index = 0; index < aieCDOGroup.pre_cdo_groups.size; index++) {
+  const uint64_t* aiePreCDOGroupArray = reinterpret_cast<const uint64_t*>(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<const cdo_group*>(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<const cdo_group*>(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<const char*>(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<const uint64_t*>(pBase + element.dpu_kernel_ids.offset);
-      for (uint32_t kernelIDindex = 0; kernelIDindex < element.dpu_kernel_ids.size; kernelIDindex++) {
+      const uint64_t* kernelIDsArray = reinterpret_cast<const uint64_t*>(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,34 +614,47 @@ 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<const char*>(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)
 {
   XUtil::TRACE("Populating DPI Array");
   boost::property_tree::ptree ptPDIArray;
 
-  const aie_pdi* aiePdiArray = reinterpret_cast<const aie_pdi*>(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<const aie_pdi*>(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;

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<const char*>(&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
   }

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);
 
 

src/runtime_src/tools/xclbinutil/SectionDebugIPLayout.cxx

@@ -198,7 +198,7 @@ SectionDebugIPLayout::marshalToJSON(char* _pDataSection,
                  % static_cast<unsigned int>(pHdr->m_debug_ip_data[index].m_major)
                  % static_cast<unsigned int>(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<const char*>(&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<unsigned int>(pHdr->m_debug_ip_data[index].m_major)).str());
     debug_ip_data.put("m_minor", (boost::format("%d") % static_cast<unsigned int>(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
   }

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

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
   }

src/runtime_src/tools/xclbinutil/SectionMCS.cxx

@@ -242,21 +242,21 @@ SectionMCS::extractBuffers(const char* _pDataSection,
 
     XUtil::TRACE_BUF("m_chunk", reinterpret_cast<const char*>(&(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);
   }