Skip to content

Commit 48dc9ea

Browse files
brianeggeclaude
andcommitted
Check arity before printing a fileref in its special spelling (Covered by dco/Brian_Egge.md)
A "fileref" type applied to one argument is printed as "t@?" and to two as "t@off". showFileRef (lang/type.C) read targs[0] as soon as the applied constructor was the "fileref" primitive, and only consulted the arity afterwards to pick between those spellings. The compiler never builds a fileref with any other arity, but the binary type decoder builds a TApp from whatever the buffer says (decodeTApp), and nothing in the encoding obliges a peer to supply an argument. Indexing the empty argument vector binds a reference through its null data pointer, and the show call through it is the null read OSS-Fuzz reports: https://issues.oss-fuzz.com/issues/550998929 - hobbes::showFileRef (fuzz-type-decode, null-dereference READ at 0x0) The harness only calls hobbes::decode, so show() is reached without leaving the decoder: the record layout validation rejects the reproducer's record for an offset mismatch, and that diagnostic renders the record it rejects, member types and all -- decodeRecord -> Record::make -> withResolvedMemoryLayout -> "Actual/calculated member offset mismatch ... in record:" -> showRecord -> Variant::show -> showFull -> TApp::show -> showFileRef -> targs[0] sizeOf's "Can't determine size of monotype" diagnostic shows the type the same way. The bug is not confined to those messages, though: a no-argument fileref decodes successfully, so any later rendering of a decoded type reaches it, and the RPC layer (ipc/net.C) does render them -- both sides feed peer-supplied type descriptions through the typechecker, whose errors print the types involved, and Client::show prints the decoded result types directly. Old bug, not a regression: showFileRef is unchanged since the initial import (f81d016, 2017), so every release carries it. The layout diagnostics that let a bare decode() trip over it are recent (d45c864), but the RPC path could always reach it by showing a decoded type. The sibling special-case printer, Constraint::show, does not share the defect -- it checks mts.size() before indexing; showFileRef was the only unguarded one. The fix: use the special spelling only when the arity fits it -- one or two arguments -- and otherwise fall through to the generic constructor form, so a no-argument application prints as "(fileref)". This also corrects the over-applied case, which used to print "t@" and silently drop the extra arguments. TypeInf/ShowToleratesFileRefWithoutArguments drives everything through hobbes::decode on encoded bytes, the surface the harness and the RPC layer consume: a no-argument application shows in constructor form, an over-applied one shows all its arguments, a record shaped like the reproducer (a variant member carrying the no-argument application, at a rejected offset) throws cleanly instead of crashing while the rejection message is built, and the one- and two-argument spellings are unchanged. It fails with the fix reverted (SIGSEGV). The minimized reproducer joins the fuzz seed corpus in fuzz/corpus/type-decode/, which oss-fuzz-build.sh now packages like the other corpora. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Cf5Cv9r1wqa2YcgCWoLC3
1 parent 3d08bc2 commit 48dc9ea

4 files changed

Lines changed: 71 additions & 2 deletions

File tree

302 Bytes
Binary file not shown.

fuzz/oss-fuzz-build.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,7 @@ fi
102102
if [ -d "$SRCDIR/fuzz/corpus/fregion-reader" ]; then
103103
zip -jq "$OUT/fuzz-fregion-reader_seed_corpus.zip" "$SRCDIR/fuzz/corpus/fregion-reader/"*
104104
fi
105+
106+
if [ -d "$SRCDIR/fuzz/corpus/type-decode" ]; then
107+
zip -jq "$OUT/fuzz-type-decode_seed_corpus.zip" "$SRCDIR/fuzz/corpus/type-decode/"*
108+
fi

lib/hobbes/lang/type.C

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,12 +676,14 @@ const MonoTypePtr& TAbs::body() const { return this->b; }
676676
// TODO: come up with a nicer way to generically print types in constructor form
677677
bool showFileRef(const MonoTypePtr& f, const MonoTypes& targs, std::ostream& out) {
678678
if (const Prim* fn = is<Prim>(f)) {
679-
if (fn->name() == "fileref") {
679+
// a decoded type description can apply "fileref" to any number of
680+
// arguments, not just the one or two this spelling has slots for
681+
if (fn->name() == "fileref" && (targs.size() == 1 || targs.size() == 2)) {
680682
targs[0]->show(out);
681683
out << "@";
682684
if (targs.size() == 1) {
683685
out << "?";
684-
} else if (targs.size() == 2) {
686+
} else {
685687
targs[1]->show(out);
686688
}
687689
return true;

test/TypeInf.C

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,69 @@ TEST(TypeInf, DecodeRejectsNonPrimitiveSwitchSelector) {
411411
EXPECT_TRUE(*rt == *sw);
412412
}
413413

414+
TEST(TypeInf, ShowToleratesFileRefWithoutArguments) {
415+
// "fileref" applied to one or two arguments is printed in a special spelling
416+
// ("t@?", "t@off"), and showFileRef read its first argument before asking how
417+
// many there were. The compiler only ever applies "fileref" to one or two
418+
// arguments, but the binary decoder builds a TApp from whatever the buffer
419+
// says, and nothing in the encoding obliges a peer to supply any -- so a
420+
// decoded application of "fileref" to nothing indexed an empty vector.
421+
//
422+
// OSS-Fuzz 550998929 reaches show() without leaving the decoder: the record
423+
// layout diagnostics added with SizeAndLayoutRejectUnrepresentableTypes above
424+
// render the record they reject, member types and all. But any rendering of a
425+
// decoded type gets there too, e.g. a type error against a peer-supplied
426+
// type description (ipc/net.C feeds both sides' decoded types to the
427+
// typechecker, whose errors print them).
428+
429+
// an application of "fileref" to no arguments, arriving as encoded bytes the
430+
// way the fuzz harness and the RPC layer receive it, falls back to the
431+
// generic constructor spelling instead of reading absent arguments
432+
std::vector<unsigned char> noargs;
433+
encode(MonoTypePtr(TApp::make(primty("fileref"), MonoTypes())), &noargs);
434+
EXPECT_EQ(show(decode(noargs)), "(fileref)");
435+
436+
// more arguments than the spelling has slots for fall back the same way
437+
// (before, the extras were silently dropped rather than shown)
438+
std::vector<unsigned char> extra;
439+
encode(MonoTypePtr(TApp::make(primty("fileref"), list(primty("int"), primty("long"), primty("char")))), &extra);
440+
EXPECT_EQ(show(decode(extra)), "(fileref int long char)");
441+
442+
// the reproducer's own shape: a record whose member -- a variant carrying the
443+
// no-argument application -- has an offset the layout check rejects, where
444+
// the rejection message renders the record; it must throw, not crash
445+
const MonoTypePtr payload(TApp::make(primty("fileref"), MonoTypes()));
446+
Variant::Members vms;
447+
vms.push_back(Variant::Member(".f0", payload, 0));
448+
449+
std::vector<unsigned char> rec;
450+
auto put = [&](const void* p, size_t n) {
451+
const auto* b = static_cast<const unsigned char*>(p);
452+
rec.insert(rec.end(), b, b + n);
453+
};
454+
const int tcode = Record::type_case_id;
455+
const size_t nmems = 1;
456+
const size_t namesz = 1;
457+
const unsigned int offset = 2; // calculated offset is 0; mismatch is rejected
458+
put(&tcode, sizeof(tcode));
459+
put(&nmems, sizeof(nmems));
460+
put(&namesz, sizeof(namesz));
461+
put("a", 1);
462+
put(&offset, sizeof(offset));
463+
encode(MonoTypePtr(Variant::make(vms)), &rec);
464+
465+
bool threw = false;
466+
try { decode(rec); } catch (const std::exception&) { threw = true; }
467+
EXPECT_TRUE(threw);
468+
469+
// the spellings for one and two arguments are unchanged
470+
std::vector<unsigned char> one, two;
471+
encode(fileRefTy(primty("int")), &one);
472+
EXPECT_EQ(show(decode(one)), "int@?");
473+
encode(fileRefTy(primty("int"), tlong(42)), &two);
474+
EXPECT_EQ(show(decode(two)), "int@42L");
475+
}
476+
414477
TEST(TypeInf, SubstitutionTerminatesOnCyclicSubstitutions) {
415478
// substitution is applied to its fixed point, which only exists while no variable
416479
// stands for a type that mentions it -- unification maintains that, but a

0 commit comments

Comments
 (0)