From 9f9e4e28b851e7e440ac2835727d1c4a2d326c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Ortellado?= Date: Mon, 5 Oct 2026 08:44:55 -0300 Subject: [PATCH] fix(relinker): reject invalid and overflowing alignment --- core/relinker/io/include/io/BufferUtils.hpp | 11 +++++- core/relinker/io/src/BufferUtils.cpp | 6 +-- core/relinker/io/tests/BufferBoundsTests.cpp | 41 ++++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/core/relinker/io/include/io/BufferUtils.hpp b/core/relinker/io/include/io/BufferUtils.hpp index 365a50847..82cc8d0ec 100644 --- a/core/relinker/io/include/io/BufferUtils.hpp +++ b/core/relinker/io/include/io/BufferUtils.hpp @@ -2,6 +2,8 @@ #define IO_BUFFERUTILS_HPP #include +#include +#include #include #include @@ -27,7 +29,14 @@ std::uint64_t ReadU64(const std::vector& buf, std::size_t offset); template constexpr T AlignUp(T value, T alignment) { - return (value + alignment - 1) / alignment * alignment; + if (value < 0 || alignment <= 0) + throw std::invalid_argument("AlignUp requires a nonnegative value and a positive alignment"); + const T remainder = value % alignment; + if (remainder == 0) return value; + const T padding = alignment - remainder; + if (value > std::numeric_limits::max() - padding) + throw std::overflow_error("Aligned value exceeds integer range"); + return value + padding; } std::uint64_t AlignUp64(std::uint64_t value, std::uint64_t alignment); diff --git a/core/relinker/io/src/BufferUtils.cpp b/core/relinker/io/src/BufferUtils.cpp index c0d389588..d4ce96336 100644 --- a/core/relinker/io/src/BufferUtils.cpp +++ b/core/relinker/io/src/BufferUtils.cpp @@ -60,11 +60,7 @@ void AppendString(std::vector& buf, const std::string& str) { } void AlignBuffer(std::vector& buf, std::size_t alignment) { - const std::size_t remainder = buf.size() % alignment; - if (remainder != 0) { - const std::size_t padding = alignment - remainder; - buf.resize(buf.size() + padding, 0); - } + buf.resize(AlignUp(buf.size(), alignment), 0); } std::uint64_t AlignUp64(std::uint64_t value, std::uint64_t alignment) { diff --git a/core/relinker/io/tests/BufferBoundsTests.cpp b/core/relinker/io/tests/BufferBoundsTests.cpp index 2de9beb8b..6767291ff 100644 --- a/core/relinker/io/tests/BufferBoundsTests.cpp +++ b/core/relinker/io/tests/BufferBoundsTests.cpp @@ -11,6 +11,8 @@ namespace { using Bytes = std::vector; +static_assert(Io::AlignUp(std::uint64_t{17}, std::uint64_t{16}) == 32); + void require(bool condition, const std::string& message) { if (!condition) throw std::runtime_error(message); } @@ -71,12 +73,51 @@ void writeBounds(const TWrite& write, const std::string& name) { invalidRanges(bytes, [&](Bytes& buffer, std::size_t offset) { write(buffer, offset, value); }, name); } +template +void requireThrows(const TOperation& operation, const std::string& message) { + try { + operation(); + } catch (const TError&) { + return; + } + throw std::runtime_error(message); +} + +template +void alignmentBounds() { + constexpr auto maximum = std::numeric_limits::max(); + require(Io::AlignUp(TValue{0}, TValue{16}) == 0, "AlignUp changed zero"); + require(Io::AlignUp(TValue{17}, TValue{16}) == 32, "AlignUp failed to round up"); + require(Io::AlignUp(TValue{7}, TValue{3}) == 9, "AlignUp requires a power-of-two alignment"); + require(Io::AlignUp(maximum, TValue{1}) == maximum, "AlignUp changed an aligned maximum"); + require(Io::AlignUp(static_cast(maximum - 15), TValue{16}) == maximum - 15, "AlignUp changed an aligned value near the maximum"); + require(Io::AlignUp(static_cast(maximum - 1), maximum) == maximum, "AlignUp overflowed for a representable result"); + requireThrows([&] { Io::AlignUp(maximum, TValue{16}); }, "AlignUp accepted an overflowing result"); + requireThrows([] { Io::AlignUp(TValue{1}, TValue{0}); }, "AlignUp accepted zero alignment"); +} + } int main() { try { const Io::ByteReader reader; const Io::ByteWriter writer; + alignmentBounds(); + alignmentBounds(); + alignmentBounds(); + alignmentBounds(); + requireThrows([] { Io::AlignUp(-1, 16); }, "AlignUp accepted a negative value"); + requireThrows([] { Io::AlignUp(1, -1); }, "AlignUp accepted negative alignment"); + require(Io::AlignUp64(std::numeric_limits::max() - 1, std::numeric_limits::max()) == std::numeric_limits::max(), "AlignUp64 overflowed for a representable result"); + requireThrows([] { Io::AlignUp64(std::numeric_limits::max(), 16); }, "AlignUp64 accepted an overflowing result"); + Bytes aligned{1, 2, 3}; + const auto original = aligned; + requireThrows([&] { Io::AlignBuffer(aligned, 0); }, "AlignBuffer accepted zero alignment"); + require(aligned == original, "AlignBuffer modified the buffer on failure"); + Io::AlignBuffer(aligned, 4); + require(aligned == Bytes({1, 2, 3, 0}), "AlignBuffer failed to preserve bytes and zero-fill padding"); + Io::AlignBuffer(aligned, 4); + require(aligned == Bytes({1, 2, 3, 0}), "AlignBuffer changed an aligned buffer"); readBounds([&](const Bytes& bytes, std::size_t offset) { return reader.ReadU16(bytes, offset); }, "ByteReader::ReadU16"); readBounds([&](const Bytes& bytes, std::size_t offset) { return reader.ReadU32(bytes, offset); }, "ByteReader::ReadU32"); readBounds([&](const Bytes& bytes, std::size_t offset) { return reader.ReadU64(bytes, offset); }, "ByteReader::ReadU64");