Skip to content

SWSPLAT-30704 firmware_image fix security issues in xsabin parsing #9889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stsoe merged 1 commit into from
Jul 2, 2026
+93 −44
Merged

SWSPLAT-30704 firmware_image fix security issues in xsabin parsing #9889

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 93 additions & 44 deletions src/runtime_src/core/tools/xbmgmt2/flash/firmware_image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,23 @@
/**
* Copyright (C) 2019-2022 Xilinx, Inc
* Copyright (C) 2022 Advanced Micro Devices, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may
* not use this file except in compliance with the License. A copy of the
* License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

// SPDX-License-Identifier: Apache-2.0
// Copyright (C) 2019-2022 Xilinx, Inc
// Copyright (C) 2022-2026 Advanced Micro Devices, Inc. - All rights reserved
#include "firmware_image.h"
#include "core/common/utils.h"
#include "core/include/xrt/detail/xclbin.h"

// 3rd Party Library - Include Files
#include <boost/format.hpp>
#include <boost/algorithm/string.hpp>
#include <boost/tokenizer.hpp>

#include <iostream>
#include <climits>
#include <cstdint>
#include <cstring>
#include <algorithm>
#include <filesystem>
#include <fstream>
#include <algorithm>
#include <climits>
#include <iomanip>
#include <iostream>
#include <memory>
#include <cstdint>
#include <cstring>
#include <vector>

#ifdef _WIN32
Expand Down Expand Up @@ -175,6 +160,16 @@ void getUUIDFromDTB(void *blob, uint64_t &ts, std::vector<std::string> &uuids)
uuid2ts(uuids[0], ts);
}

// Verify that [offset, offset+size) fits within a buffer of known length.
// Throws on overflow or out-of-range access.
static void
check_section_bounds(uint64_t offset, uint64_t size, uint64_t buf_len,
const char *ctx)
{
if (offset > buf_len || size > buf_len - offset)
throw std::runtime_error(std::string(ctx) + ": section offset/size out of range");
}

DSAInfo::DSAInfo(const std::string& filename, uint64_t ts, const std::string& id, const std::string& bmcV) :
hasFlashImage(false), vendor(), board(), name(), file(filename),
timestamp(ts), bmcVer(bmcV),
Expand Down Expand Up @@ -301,6 +296,8 @@ DSAInfo::DSAInfo(const std::string& filename, uint64_t ts, const std::string& id
// Show it as ID for flashing
const axlf_section_header* dtbSection = xclbin::get_axlf_section(ap, PARTITION_METADATA);
if (dtbSection && timestamp == NULL_TIMESTAMP) {
check_section_bounds(dtbSection->m_sectionOffset, dtbSection->m_sectionSize,
ap->m_header.m_length, "DTB section");
std::vector<char> dtbbuf(dtbSection->m_sectionSize);
in.seekg(dtbSection->m_sectionOffset);
in.read(dtbbuf.data(), dtbSection->m_sectionSize);
Expand All @@ -317,6 +314,8 @@ DSAInfo::DSAInfo(const std::string& filename, uint64_t ts, const std::string& id
if (bmcSection == nullptr)
return;
// Load entire BMC section.
check_section_bounds(bmcSection->m_sectionOffset, bmcSection->m_sectionSize,
ap->m_header.m_length, "BMC section (DSAInfo)");
std::vector<char> bmcbuf(bmcSection->m_sectionSize);
in.seekg(bmcSection->m_sectionOffset);
in.read(bmcbuf.data(), bmcSection->m_sectionSize);
Expand Down Expand Up @@ -367,7 +366,7 @@ bool DSAInfo::matchId(const std::string &id) const
if (ts != 0 && ts != ULLONG_MAX && ts == timestamp)
return true;

if (uuids.empty()) {
if (!uuids.empty()) {
const std::string uuid = normalize_uuid(id);

if (!strncmp(uuids[0].c_str(), uuid.c_str(), uuid.length()))
Expand Down Expand Up @@ -469,29 +468,36 @@ remove_xsabin_mirror(void * xsabin_buffer)
axlf *axlf_header = reinterpret_cast<struct axlf *>(xsabin_buffer);
uint64_t bufferSize = axlf_header->m_header.m_length;

std::stringstream strm;
strm << xsabin_buffer;
std::string str_xsabin_buffer = strm.str();
auto *buf_begin = reinterpret_cast<unsigned char *>(xsabin_buffer);
auto *buf_end = buf_begin + bufferSize;

auto start_offset = str_xsabin_buffer.find(mirror_data_start);
if (start_offset == std::string::npos)
auto *start_ptr = std::search(buf_begin, buf_end,
mirror_data_start.begin(), mirror_data_start.end());
if (start_ptr == buf_end)
return; // No MIRROR DATA

auto end_offset = str_xsabin_buffer.find(mirror_data_end);
if (end_offset == std::string::npos)
auto *end_ptr = std::search(buf_begin, buf_end,
mirror_data_end.begin(), mirror_data_end.end());
if (end_ptr == buf_end)
return; // Badly formed mirror data (we have a start, but no end)

if (end_offset <= start_offset)
return; // Tags are in the wrong order
if (end_ptr <= start_ptr)
return; // Tags are in the wrong order

// Zero out memory (not really needed but done for completeness)
uint64_t start_offset = static_cast<uint64_t>(start_ptr - buf_begin);

@github-actions github-actions Bot Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use auto when initializing with a cast to avoid duplicating the type name [hicpp-use-auto]

Suggested change
uint64_t start_offset = static_cast<uint64_t>(start_ptr - buf_begin);
auto start_offset = static_cast<uint64_t>(start_ptr - buf_begin);

uint64_t end_offset = static_cast<uint64_t>(end_ptr - buf_begin);

@github-actions github-actions Bot Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use auto when initializing with a cast to avoid duplicating the type name [hicpp-use-auto]

Suggested change
uint64_t end_offset = static_cast<uint64_t>(end_ptr - buf_begin);
auto end_offset = static_cast<uint64_t>(end_ptr - buf_begin);


// Both offsets are within [0, bufferSize) by construction (search returned
// pointers inside [buf_begin, buf_end)), so no further range check needed.
uint64_t bytesRemoved = end_offset - start_offset;
std::memset(reinterpret_cast<unsigned char *>(xsabin_buffer) + start_offset, 0, bytesRemoved);
uint64_t bytesToCopy = bufferSize - end_offset;

// Zero out memory (not really needed but done for completeness)
std::memset(buf_begin + start_offset, 0, bytesRemoved);

// Compress the image
uint64_t bytesToCopy = bufferSize - end_offset;
if (bytesToCopy != 0)
std::memcpy(reinterpret_cast<unsigned char *>(xsabin_buffer) + start_offset, reinterpret_cast<unsigned char *>(xsabin_buffer) + end_offset, bytesToCopy);
std::memcpy(buf_begin + start_offset, buf_begin + end_offset, bytesToCopy);

// Update length of the buffer
axlf_header->m_header.m_length = bufferSize - bytesRemoved;
Expand All @@ -514,20 +520,27 @@ remove_xsabin_section(void * xsabin_buffer, enum axlf_section_kind section_to_re
// Is this a section of interest, if not then go to the next section
if (sectionHeaderArray[index].m_sectionKind != static_cast<uint32_t>(section_to_remove))
continue;

// Record the buffer size
uint64_t bufferSize = axlf_header->m_header.m_length;

// Determine the data to be removed.
uint64_t startToOffset = sectionHeaderArray[index].m_sectionOffset;
uint64_t startFromOffset = ((index + 1) == axlf_header->m_header.m_numSections) ?
sectionHeaderArray[index].m_sectionOffset + sectionHeaderArray[index].m_sectionSize:
sectionHeaderArray[index + 1].m_sectionOffset;
uint64_t startFromOffset = ((index + 1) == axlf_header->m_header.m_numSections)
? sectionHeaderArray[index].m_sectionOffset + sectionHeaderArray[index].m_sectionSize
: sectionHeaderArray[index + 1].m_sectionOffset;

if (startToOffset > bufferSize
|| startFromOffset < startToOffset
|| startFromOffset > bufferSize)
throw std::runtime_error("xsabin: section offset/size out of range");

uint64_t bytesToCopy = bufferSize - startFromOffset;
uint64_t bytesRemoved = startFromOffset - startToOffset;

if (bytesToCopy != 0) {
if (bytesToCopy) {
std::memcpy(reinterpret_cast<unsigned char *>(xsabin_buffer) + startToOffset,
reinterpret_cast<unsigned char *>(xsabin_buffer) + startFromOffset, bytesToCopy);
reinterpret_cast<unsigned char *>(xsabin_buffer) + startFromOffset, bytesToCopy);
}
// -- Now do some incremental clean up of the data structures
// Update the length and offsets AFTER this entry
Expand All @@ -546,8 +559,14 @@ remove_xsabin_section(void * xsabin_buffer, enum axlf_section_kind section_to_re
// Remove the array entry
void * ptrStartTo = &sectionHeaderArray[index];
void * ptrStartFrom = &sectionHeaderArray[index+1];
uint64_t bytesToShift = axlf_header->m_header.m_length -
(reinterpret_cast<unsigned char *>(ptrStartFrom) - reinterpret_cast<unsigned char *>(xsabin_buffer));
auto fromOffset = static_cast<uint64_t>(
reinterpret_cast<unsigned char *>(ptrStartFrom) -
reinterpret_cast<unsigned char *>(xsabin_buffer));

if (fromOffset > axlf_header->m_header.m_length)
throw std::runtime_error("xsabin: section header array extends beyond buffer");

uint64_t bytesToShift = axlf_header->m_header.m_length - fromOffset;
std::memcpy(reinterpret_cast<unsigned char *>(ptrStartTo), reinterpret_cast<unsigned char *>(ptrStartFrom), bytesToShift);

// Update data elements
Expand Down Expand Up @@ -616,6 +635,13 @@ firmwareImage::firmwareImage(const std::string& file, imageType type) :
return;
}
// Load entire BMC section.
check_section_bounds(bmcSection->m_sectionOffset, bmcSection->m_sectionSize,
ap->m_header.m_length, "BMC section");
if (bmcSection->m_sectionSize < sizeof(struct bmc)) {
this->setstate(failbit);
std::cout << "BMC section too small in "<< file << std::endl;
return;
}
std::vector<char> bmcbuf(bmcSection->m_sectionSize);
in_file.seekg(bmcSection->m_sectionOffset);
in_file.read(bmcbuf.data(), bmcSection->m_sectionSize);
Expand All @@ -626,6 +652,9 @@ firmwareImage::firmwareImage(const std::string& file, imageType type) :
return;
}
const struct bmc *bmc = reinterpret_cast<const struct bmc *>(bmcbuf.data());
// Validate embedded offset/size against the section bounds.
check_section_bounds(bmc->m_offset, bmc->m_size,
bmcSection->m_sectionSize, "BMC image within section");
// Load data into stream.
bufsize = bmc->m_size;
mBuf = new char[bufsize];
Expand Down Expand Up @@ -674,6 +703,13 @@ firmwareImage::firmwareImage(const std::string& file, imageType type) :
}

//load 'struct flash'
check_section_bounds(flashSection->m_sectionOffset, flashSection->m_sectionSize,
ap->m_header.m_length, "FLASH section");
if (flashSection->m_sectionSize < sizeof(struct flash)) {
this->setstate(failbit);
std::cout << "FLASH section too small in "<< file << std::endl;
return;
}
struct flash flashMeta;
in_file.seekg(flashSection->m_sectionOffset);
in_file.read(reinterpret_cast<char *>(&flashMeta), sizeof(flashMeta));
Expand All @@ -683,6 +719,9 @@ firmwareImage::firmwareImage(const std::string& file, imageType type) :
std::cout << "Can't read FLASH section from "<< file << std::endl;
return;
}
// Validate embedded image offset/size against the section bounds.
check_section_bounds(flashMeta.m_image_offset, flashMeta.m_image_size,
flashSection->m_sectionSize, "flash image within section");
// Load data into stream.
bufsize = flashMeta.m_image_size;
mBuf = new char[bufsize];
Expand Down Expand Up @@ -716,6 +755,13 @@ firmwareImage::firmwareImage(const std::string& file, imageType type) :
return;
}
// Load entire MCS section.
check_section_bounds(mcsSection->m_sectionOffset, mcsSection->m_sectionSize,
ap->m_header.m_length, "MCS section");
if (mcsSection->m_sectionSize < sizeof(struct mcs)) {
this->setstate(failbit);
std::cout << "MCS section too small in "<< file << std::endl;
return;
}
std::vector<char> mcsbuf(mcsSection->m_sectionSize);
in_file.seekg(mcsSection->m_sectionOffset);
in_file.read(mcsbuf.data(), mcsSection->m_sectionSize);
Expand All @@ -742,6 +788,9 @@ firmwareImage::firmwareImage(const std::string& file, imageType type) :
this->setstate(failbit);
return;
}
// Validate embedded chunk offset/size against the section bounds.
check_section_bounds(c->m_offset, c->m_size,
mcsSection->m_sectionSize, "MCS chunk within section");
// Load data into stream.
bufsize = c->m_size;
mBuf = new char[bufsize];
Expand Down
Loading