swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[MG_State, MG_Test] (P7): PH-5 bounds a vector count by its element's minimum encoded size, so compact archives the codec writes decode again

Browse Source
This commit is contained in:
swung0x48
2026-09-23 17:04:13 -04:00
parent 6afa077f96
commit 1052b13a1a
3 changed files with 185 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_State/GLState/ProgramState/ProgramArtifactsCodec.cpp
+61 -9
View File
@@ -247,16 +247,68 @@ namespace MobileGL::MG_State::GLState {
return true;
}
// A COUNT IS CHECKED AGAINST THE BYTES THAT REMAIN BEFORE ANYTHING IS ALLOCATED.
// For resizable vectors, also charge sizeof(value_type) per element: even when a
// serialized record is compact, its immediate vector reservation must not exceed the
// remaining untrusted archive bytes. The division form avoids multiplying attacker
// input and is safe on both 32-bit and 64-bit SizeT.
Bool TakeCount(ReadCursor& in, SizeT& count, SizeT allocationBytesPerElement = 1) {
// ---- the fewest bytes the writer can emit for one value of a type ----
//
// What PH-5's vector bound charges per element (TakeCount below). It is the WRITER'S
// floor, derived arm for arm from WriteValue above: a scalar is its own width, a string
// or a container is its u64 count and nothing else when empty, a fixed array is its
// width times its element's floor, and a record - VisitFields, or the hand-written
// TUniformInitializer table - is the sum of its fields' floors. It depends on the TYPE
// only; the default-constructed instance is there because VisitFields needs something to
// walk, and no member value is read.
template <class Value>
SizeT MinEncodedBytesOf() {
using T = std::remove_cv_t<Value>;
if constexpr (ArchiveScalar<T>) {
return sizeof(T);
} else if constexpr (ArchiveString<T> || ArchiveMap<T> || ArchiveSet<T> || ArchiveVector<T>) {
return sizeof(Uint64);
} else if constexpr (ArchiveFixedArray<T>) {
return std::tuple_size<T>::value * MinEncodedBytesOf<typename T::value_type>();
} else {
T value{};
SizeT bytes = 0;
const auto field = [&bytes](const char*, const auto& member) {
bytes += MinEncodedBytesOf<std::remove_cvref_t<decltype(member)>>();
};
if constexpr (ArchiveUniformInitializer<T>) VisitUniformInitializer(value, field);
else VisitFields(value, field);
return bytes;
}
}
// At least one byte, so a count is always bounded by the bytes that remain (a type whose
// floor were zero - an empty record, a zero-width array - would otherwise admit any count).
template <class Value>
SizeT MinEncodedBytes() {
static const SizeT bytes = [] {
const SizeT least = MinEncodedBytesOf<Value>();
return least == 0 ? SizeT{1} : least;
}();
return bytes;
}
// A COUNT IS CHECKED AGAINST THE BYTES THAT REMAIN BEFORE ANYTHING IS ALLOCATED: every
// one of `count` elements still has to be read out of Remaining(), so a count larger than
// Remaining() / (the element's smallest encoding) is one no stream this writer produced
// can back, and it is refused before the resize.
//
// PH-5 (codex closeout finding 3): the charge per element is the MINIMUM ENCODED size,
// NOT sizeof(value_type). The first PH-5 charged sizeof, and that refused archives this
// very encoder writes: a Vector<String> element is eight bytes of count plus its
// characters on the wire but a 32-byte object in memory, so ten one-character
// xfbInterfaceNames with empty vectors behind them decoded to false
// (ProgramArtifactsCodecTest's ACompactArchiveOf* round trips). The allocation stays
// bounded all the same - resize(count) costs at most
// (Remaining() / MinEncodedBytes<T>()) * sizeof(T), a
// constant factor of the archive fixed per type (on libstdc++: 4 for a String, 3 for a
// nested vector, ~1.3 for a ResourceReflection), never the peer's count. The division
// form avoids multiplying attacker input and is safe on both 32-bit and 64-bit SizeT.
Bool TakeCount(ReadCursor& in, SizeT& count, SizeT minEncodedBytesPerElement = 1) {
Uint64 raw = 0;
if (!TakeRaw(in, raw)) return false;
if (allocationBytesPerElement == 0 ||
raw > static_cast<Uint64>(in.Remaining() / allocationBytesPerElement)) {
if (minEncodedBytesPerElement == 0 ||
raw > static_cast<Uint64>(in.Remaining() / minEncodedBytesPerElement)) {
in.Ok = false;
return false;
}
@@ -306,7 +358,7 @@ namespace MobileGL::MG_State::GLState {
} else if constexpr (ArchiveVector<T>) {
SizeT count = 0;
using Element = typename T::value_type;
if (!TakeCount(in, count, sizeof(Element))) return;
if (!TakeCount(in, count, MinEncodedBytes<Element>())) return;
value.clear();
value.resize(count);
for (auto& element : value) {
MobileGL/MG_Test/Program/ProgramArtifactsCodecTest.cpp
+119 -9
View File
@@ -193,6 +193,36 @@ namespace {
return link;
}
// How many archive bytes follow a u64 `count` that is itself followed by `next` - the LAST such
// spot, so a fixture can find the count of a field near the archive's end. This is
// Remaining() at the moment TakeCount reads that count. SIZE_MAX when there is no such spot.
SizeT BytesAfterTheCount(const Vector<Uint8>& bytes, Uint64 count, const Vector<Uint8>& next) {
Vector<Uint8> needle(sizeof(count));
std::memcpy(needle.data(), &count, sizeof(count));
needle.insert(needle.end(), next.begin(), next.end());
if (bytes.size() < needle.size()) return static_cast<SizeT>(-1);
for (SizeT at = bytes.size() - needle.size() + 1; at-- > 0;) {
if (std::memcmp(bytes.data() + at, needle.data(), needle.size()) == 0) {
return bytes.size() - at - sizeof(count);
}
}
return static_cast<SizeT>(-1);
}
// The encoded width of one DEFAULT element of LinkArtifacts::uniformReflection, measured
// through the encoder itself: an archive with one such element minus an archive with none.
// A default ResourceReflection has an empty name, so this is the smallest encoding one can
// have - the charge PH-5's TakeCount makes per element.
SizeT EncodedBytesOfOneDefaultResourceReflection() {
Vector<Uint8> none;
EncodeProgramArtifacts(LinkArtifacts{}, SpirvArtifacts{}, none);
LinkArtifacts one{};
one.uniformReflection.resize(1);
Vector<Uint8> withOne;
EncodeProgramArtifacts(one, SpirvArtifacts{}, withOne);
return withOne.size() - none.size();
}
SpirvArtifacts MakeSpirvArtifacts() {
SpirvArtifacts spirv{};
spirv.generatedSpirv = {{0x07230203u, 0x00010300u, 0u}, {0x07230203u, 0x00010300u, 1u}};
@@ -299,6 +329,75 @@ TEST(ProgramArtifactsCodec, RoundTripsAFullyPopulatedArchive) {
#endif
}
// A COMPACT archive - one whose wire form is SMALLER than its in-memory form - is still an archive
// this encoder wrote, and it has to come back (codex closeout finding 3). Ten one-character
// xfbInterfaceNames are 90 bytes on the wire and ten 32-byte std::strings in memory; with every
// vector behind them empty, the bytes that remain at their count are fewer than 10 x sizeof(String),
// and PH-5's first bound - which charged sizeof(value_type) per element - refused the archive. The
// same holds one level down for SpirvArtifacts::generatedSpirv: ten EMPTY modules are ten u64
// counts on the wire and ten 24-byte vectors in memory (the next case). Red with TakeCount's vector
// charge put back to sizeof(Element): the decode answers false. Each case's ASSERT_LT is the
// fixture's own proof that it can go red that way.
TEST(ProgramArtifactsCodec, ACompactArchiveOfTenShortXfbNamesRoundTrips) {
#if MOBILEGL_PIPE_PUSH
{
LinkArtifacts link{};
link.xfbInterfaceNames = {"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"};
Vector<Uint8> bytes;
EncodeProgramArtifacts(link, SpirvArtifacts{}, bytes);
const Uint64 one = 1;
Vector<Uint8> firstName(sizeof(one));
std::memcpy(firstName.data(), &one, sizeof(one));
firstName.push_back(static_cast<Uint8>('a'));
const SizeT remaining = BytesAfterTheCount(bytes, link.xfbInterfaceNames.size(), firstName);
ASSERT_LT(remaining, link.xfbInterfaceNames.size() * sizeof(String))
<< "fixture: a sizeof(value_type) charge would admit this count, so the case could not go red";
LinkArtifacts decodedLink;
SpirvArtifacts decodedSpirv;
ASSERT_TRUE(DecodeProgramArtifacts(bytes.data(), bytes.size(), decodedLink, decodedSpirv))
<< "a LinkArtifacts with ten short XFB interface names, which this encoder wrote, was refused";
EXPECT_EQ(decodedLink.xfbInterfaceNames, link.xfbInterfaceNames);
EXPECT_TRUE(decodedLink.xfbStrides.empty());
EXPECT_TRUE(decodedLink.gsStripTriangles.empty());
Vector<Uint8> reencoded;
EncodeProgramArtifacts(decodedLink, decodedSpirv, reencoded);
EXPECT_EQ(reencoded, bytes);
}
#else
GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: the archive codec is push-only";
#endif
}
// The same compact shape one level down: a Vector<Vector<unsigned>> element (one SPIR-V module) is
// its u64 count on the wire when empty and a 24-byte object in memory.
TEST(ProgramArtifactsCodec, ACompactArchiveOfTenEmptySpirvModulesRoundTrips) {
#if MOBILEGL_PIPE_PUSH
{
SpirvArtifacts spirv{};
spirv.generatedSpirv.resize(10);
Vector<Uint8> bytes;
EncodeProgramArtifacts(LinkArtifacts{}, spirv, bytes);
const SizeT remaining = BytesAfterTheCount(bytes, spirv.generatedSpirv.size(),
Vector<Uint8>(10 * sizeof(Uint64), 0));
ASSERT_LT(remaining, spirv.generatedSpirv.size() * sizeof(Vector<unsigned>))
<< "fixture: a sizeof(value_type) charge would admit this count, so the case could not go red";
LinkArtifacts decodedLink;
SpirvArtifacts decodedSpirv;
ASSERT_TRUE(DecodeProgramArtifacts(bytes.data(), bytes.size(), decodedLink, decodedSpirv))
<< "a SpirvArtifacts with ten empty modules, which this encoder wrote, was refused";
ASSERT_EQ(decodedSpirv.generatedSpirv.size(), 10u);
for (const auto& module : decodedSpirv.generatedSpirv) EXPECT_TRUE(module.empty());
Vector<Uint8> reencoded;
EncodeProgramArtifacts(decodedLink, decodedSpirv, reencoded);
EXPECT_EQ(reencoded, bytes);
}
#else
GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: the archive codec is push-only";
#endif
}
// Negative control 1. Every prefix length is checked against the bytes that REMAIN, so a
// stream cut anywhere has to come back false with both outputs defaulted - never a partially
// filled archive, and never a resize driven by a count the stream cannot back.
@@ -335,8 +434,12 @@ TEST(ProgramArtifactsCodec, ATruncatedStreamIsRefusedNotGuessed) {
}
// A count is not permission to reserve a container larger than the bytes left in the
// untrusted archive. This mutates the first LinkArtifacts vector count to the boundary value
// that the former raw <= Remaining check accepted but sizeof(value_type) <= Remaining rejects.
// untrusted archive can back. This mutates the first LinkArtifacts vector count to the boundary
// value one element past PH-5's bound - the smallest count the remaining bytes cannot hold at
// the element's minimum encoded size - which the former raw <= Remaining check accepted. (Not
// discriminating on its own: under the byte bound the resize is small and the element reads run
// out of bytes, so the decode is false either way. ACountOnlyTheByteBoundAdmitsIsRefusedBeforeItsResize
// is the case that goes red.)
TEST(ProgramArtifactsCodec, AVectorCountCannotReservePastTheRemainingArchiveBytes) {
#if MOBILEGL_PIPE_PUSH
Vector<Uint8> bytes;
@@ -344,7 +447,12 @@ TEST(ProgramArtifactsCodec, AVectorCountCannotReservePastTheRemainingArchiveByte
constexpr SizeT countOffset = sizeof(Uint32) + sizeof(Uint64);
ASSERT_GT(bytes.size(), countOffset + sizeof(Uint64));
const SizeT available = bytes.size() - countOffset - sizeof(Uint64);
const Uint64 forgedCount = static_cast<Uint64>(available / sizeof(UniformReflection)) + 1;
const SizeT perElement = EncodedBytesOfOneDefaultResourceReflection();
// What finding 3 is about, stated: an element's wire floor is below its object size, and
// above the one byte the former bound charged.
ASSERT_GT(perElement, 1u);
ASSERT_LT(perElement, sizeof(UniformReflection));
const Uint64 forgedCount = static_cast<Uint64>(available / perElement) + 1;
ASSERT_LT(forgedCount, static_cast<Uint64>(available))
<< "fixture must pass the old count <= remaining check";
std::memcpy(bytes.data() + countOffset, &forgedCount, sizeof(forgedCount));
@@ -365,10 +473,12 @@ TEST(ProgramArtifactsCodec, AVectorCountCannotReservePastTheRemainingArchiveByte
// bytes - DecodeProgramArtifacts answers false either way (red-once: it stays green). What PH-5
// actually changed is the ALLOCATION a count may cause before the reads, so this case forges the
// count BETWEEN the two bounds - `count == Remaining()`, in an archive padded to 7 MiB - and
// decodes it in a forked child whose address space has 64 MiB of headroom. PH-5 refuses before
// the resize and the child exits 0; the old bound resizes to Remaining() elements of
// sizeof(UniformReflection) each, far past the headroom, and the child dies of std::bad_alloc.
// The same shape crosses the wire in PeerLatchTest's D11ArchiveVectorCount row.
// decodes it in a forked child whose address space has 64 MiB of headroom. PH-5 charges each
// element its minimum encoded size (a ResourceReflection's is ~100 bytes; the codex closeout
// finding 3 fix, not sizeof), refuses before the resize, and the child exits 0; the old bound
// resizes to Remaining() elements of sizeof(UniformReflection) each, far past the headroom, and
// the child dies of std::bad_alloc. The same shape crosses the wire in PeerLatchTest's
// D11ArchiveVectorCount row.
#if MOBILEGL_PIPE_PUSH && !defined(_WIN32)
namespace {
[[noreturn]] void DecodeACountOnlyTheByteBoundAdmitsUnderACapAndExit() {
@@ -405,8 +515,8 @@ TEST(ProgramArtifactsCodec, ACountOnlyTheByteBoundAdmitsIsRefusedBeforeItsResize
#if MOBILEGL_PIPE_PUSH && !defined(_WIN32)
EXPECT_EXIT(DecodeACountOnlyTheByteBoundAdmitsUnderACapAndExit(), ::testing::ExitedWithCode(0), ".*")
<< "std::bad_alloc escaping the child is the resize PH-5 exists to refuse: the vector count was "
"charged one byte per element instead of sizeof(value_type); exit 3 is a decode that did not "
"refuse; 64+ is the fixture";
"charged one byte per element instead of its minimum encoded size; exit 3 is a decode that "
"did not refuse; 64+ is the fixture";
#else
GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off (the archive codec is push-only), or no RLIMIT_AS / fork here";
#endif
MobileGL/MG_Test/Wire/PeerLatchTest.cpp
+5 -3
View File
@@ -750,8 +750,10 @@ namespace {
#if MOBILEGL_PIPE_PUSH
// PH-5's RAW-PEER CONTROL, MADE TO DISCRIMINATE. What PH-5 changed (ProgramArtifactsCodec.cpp
// TakeCount, 1ae7474d) is the CHARGE for a vector count: it used to be one byte per element
// (`count <= Remaining()`), and PH-5 charges sizeof(value_type) per element before the
// resize. So the control is a count BETWEEN the two bounds - one the byte bound admits and
// (`count <= Remaining()`), and PH-5 charges each element its minimum encoded size before the
// resize (sizeof(value_type) until codex closeout finding 3 showed that refused valid compact
// archives; ~100 bytes for a ResourceReflection either way). So the control is a count
// BETWEEN the two bounds - one the byte bound admits and
// PH-5 refuses: `count == Remaining()`, in an archive padded out to kArchiveBytes. Under PH-5
// the decode refuses before anything is allocated and the session latches by name. Under the
// byte bound the same count reaches `resize(count)` - Remaining() elements of
@@ -1299,7 +1301,7 @@ namespace {
// decoder's CreateShaderState.Reflection latch, but what it CONTROLS is TakeCount's
// element charge (see ArchiveWhoseCountOnlyTheByteBoundAdmits), so the site map does
// not credit it to the decoder's site (CreateShaderStateReflectionGarbage covers that).
{"D11ArchiveVectorCount", "ProgramArtifactsCodec.cpp", "PH-5 TakeCount count x sizeof(element)",
{"D11ArchiveVectorCount", "ProgramArtifactsCodec.cpp", "PH-5 TakeCount count x min encoded element",
Outcome::Latched, "Fatal{ProtocolCorruption, \"CreateShaderState.Reflection\"}", false,
[](Client::ClientSession& c, PeerReport& r) {
std::size_t element = 0;