Skip to content

[mlir][Sol] Update checks in ABI decode #41

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
vladimirradosavljevic merged 5 commits into from
Mar 18, 2026
Merged

[mlir][Sol] Update checks in ABI decode #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a0d903f
[mlir][Sol] Add missing ABI decode bounds checks for array and string…
vladimirradosavljevic Mar 16, 2026
ab8b21f
[mlir][Sol] Add explicit 64-bit tuple offset guard in decoding
vladimirradosavljevic Mar 17, 2026
43f1d9a
[mlir][Sol] Add dynamic-base innerOffset width guard in ABI decode
vladimirradosavljevic Mar 17, 2026
6517d62
[mlir][Sol] Add trailing cleanup for string type in decode
vladimirradosavljevic Mar 17, 2026
94a5ca0
[mlir][Sol] Add length guard for dynamic-memory allocation during ABI…
vladimirradosavljevic Mar 17, 2026
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
3 changes: 2 additions & 1 deletion mlir/include/mlir/Conversion/SolToStandard/EVMUtil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ class Builder {

/// Generates the memory allocation code for dynamic array.
Value genMemAllocForDynArray(Value sizeVar, Value sizeInBytes,
std::optional<Location> locArg = std::nullopt);
std::optional<Location> locArg = std::nullopt,
bool genLengthPanicGuard = false);

/// Generates the memory allocation code.
Value genMemAlloc(Type ty, bool zeroInit, ValueRange initVals, Value sizeVar,
Expand Down
108 changes: 70 additions & 38 deletions mlir/lib/Conversion/SolToStandard/EVMUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,19 @@ Value evm::Builder::genMemAlloc(AllocSize size,
}

Value evm::Builder::genMemAllocForDynArray(Value sizeVar, Value sizeInBytes,
std::optional<Location> locArg) {
std::optional<Location> locArg,
bool genLengthPanicGuard) {
Location loc = locArg ? *locArg : defLoc;

mlir::solgen::BuilderExt bExt(b, loc);

if (genLengthPanicGuard) {
auto panicCond = b.create<arith::CmpIOp>(
loc, arith::CmpIPredicate::ugt, sizeVar,
bExt.genI256Const(APInt::getLowBitsSet(256, 64)));
genPanic(mlir::evm::PanicCode::ResourceError, panicCond, loc);
}

// dynSize is size + length-slot where length-slot's size is 32 bytes.
auto dynSizeInBytes =
b.create<arith::AddIOp>(loc, sizeInBytes, bExt.genI256Const(32));
Expand Down Expand Up @@ -1583,6 +1591,23 @@ Value evm::Builder::genABITupleDecoding(Type ty, Value addr, bool fromMem,
return b.create<yul::CallDataLoadOp>(loc, addr);
};

// Revert if reading one ABI word (32 bytes) at 'addr' would exceed tuple
// bounds.
auto genRevertIfTupleWordOutOfBounds = [&](std::string const &revertMsg) {
auto invalidRangeCond = b.create<arith::CmpIOp>(
loc, arith::CmpIPredicate::sge,
b.create<arith::AddIOp>(loc, addr, bExt.genI256Const(31)), tupleEnd);
genRevertWithMsg(invalidRangeCond, revertMsg, loc);
};

// Revert if a value is past tuple end.
auto genRevertIfPastTupleEnd = [&](Value value,
std::string const &revertMsg) {
auto invalidRangeCond = b.create<arith::CmpIOp>(
loc, arith::CmpIPredicate::ugt, value, tupleEnd);
genRevertWithMsg(invalidRangeCond, revertMsg, loc);
};

// Integer type
if (auto intTy = dyn_cast<IntegerType>(ty)) {
Value arg = genLoad(addr);
Expand Down Expand Up @@ -1632,28 +1657,29 @@ Value evm::Builder::genABITupleDecoding(Type ty, Value addr, bool fromMem,
!arrTy.isDynSized())
return addr;

genRevertIfTupleWordOutOfBounds(
"ABI decoding: invalid calldata array offset");

Value dstAddr, srcAddr, size, ret;
Value thirtyTwo = bExt.genI256Const(32);
if (arrTy.isDynSized()) {
Value i256Size = genLoad(addr);
srcAddr = b.create<arith::AddIOp>(loc, addr, thirtyTwo);

// Generate an assertion that checks the size. (We don't need to do this
// for static arrays because we already generated the tuple size
// assertion).
// Generate an assertion that checks the size.
auto scaledSize = b.create<arith::MulIOp>(
loc, i256Size,
bExt.genI256Const(getCallDataHeadSize(arrTy.getEltType())));
auto endAddr = b.create<arith::AddIOp>(loc, srcAddr, scaledSize);
genRevertWithMsg(b.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ugt,
endAddr, tupleEnd),
"ABI decoding: invalid array size", loc);
genRevertIfPastTupleEnd(endAddr,
"ABI decoding: invalid calldata array stride");

if (arrTy.getDataLocation() == sol::DataLocation::CallData)
return bExt.genLLVMStruct({srcAddr, i256Size});

dstAddr = genMemAllocForDynArray(
i256Size, b.create<arith::MulIOp>(loc, i256Size, thirtyTwo));
i256Size, b.create<arith::MulIOp>(loc, i256Size, thirtyTwo), loc,
true);
ret = dstAddr;
// Skip the size fields in both the addresses.
dstAddr = b.create<arith::AddIOp>(loc, dstAddr, thirtyTwo);
Expand All @@ -1663,6 +1689,13 @@ Value evm::Builder::genABITupleDecoding(Type ty, Value addr, bool fromMem,
ret = dstAddr;
srcAddr = addr;
size = bExt.genIdxConst(arrTy.getSize());

auto fixedSize =
arrTy.getSize() * getCallDataHeadSize(arrTy.getEltType());
Value srcEnd =
b.create<arith::AddIOp>(loc, srcAddr, bExt.genI256Const(fixedSize));
genRevertIfPastTupleEnd(srcEnd,
"ABI decoding: invalid calldata array stride");
}

b.create<scf::ForOp>(
Expand All @@ -1675,14 +1708,18 @@ Value evm::Builder::genABITupleDecoding(Type ty, Value addr, bool fromMem,
Value iDstAddr = initArgs[0];
Value iSrcAddr = initArgs[1];
if (sol::hasDynamicallySizedElt(arrTy.getEltType())) {
Value innerOffset = genLoad(iSrcAddr);
auto invalidInnerOffsetCond = b.create<arith::CmpIOp>(
loc, arith::CmpIPredicate::ugt, innerOffset,
bExt.genI256Const(APInt::getLowBitsSet(256, 64)));
genRevertWithMsg(invalidInnerOffsetCond,
"ABI decoding: invalid calldata array offset",
loc);

// The elements are offset wrt to the start of this array (after the
// size field if dynamic) that contain the inner element.
Value offsetFromSrcArr =
b.create<arith::AddIOp>(loc, srcAddr, genLoad(iSrcAddr));
genRevertWithMsg(
b.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ugt,
offsetFromSrcArr, tupleEnd),
"ABI decoding: invalid array offset", loc);
b.create<arith::AddIOp>(loc, srcAddr, innerOffset);
b.create<yul::MStoreOp>(
loc, iDstAddr,
genABITupleDecoding(arrTy.getEltType(), offsetFromSrcArr,
Expand Down Expand Up @@ -1732,39 +1769,38 @@ Value evm::Builder::genABITupleDecoding(Type ty, Value addr, bool fromMem,

// String type
if (auto stringTy = dyn_cast<sol::StringType>(ty)) {
Value tailAddr = addr;
genRevertIfTupleWordOutOfBounds(
"ABI decoding: invalid calldata array offset");

Value tailAddr = addr;
Value sizeInBytes = genLoad(tailAddr);
Value thirtyTwo = bExt.genI256Const(32);
Value srcDataAddr = b.create<arith::AddIOp>(loc, tailAddr, thirtyTwo);
Value endAddr = b.create<arith::AddIOp>(loc, srcDataAddr, sizeInBytes);
genRevertWithMsg(b.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ugt,
endAddr, tupleEnd),
"ABI decoding: invalid byte array length", loc);
genRevertIfPastTupleEnd(endAddr, "ABI decoding: invalid byte array length");

if (stringTy.getDataLocation() == sol::DataLocation::CallData)
return bExt.genLLVMStruct({srcDataAddr, sizeInBytes});

// Copy the decoded string to a new memory allocation.
Value dstAddr = genMemAllocForDynArray(
sizeInBytes, bExt.genRoundUpToMultiple<32>(sizeInBytes), loc);
sizeInBytes, bExt.genRoundUpToMultiple<32>(sizeInBytes), loc, true);
Value dstDataAddr = b.create<arith::AddIOp>(loc, dstAddr, thirtyTwo);

// FIXME: ABIFunctions::abiDecodingFunctionByteArrayAvailableLength only
// allocates length + 32 (where length is rounded up to a multiple of 32)
// bytes. The "+ 32" is for the size field. But it calls
// YulUtilFunctions::copyToMemoryFunction with the _cleanup param enabled
// which makes the writing of the zero at the end an out-of-bounds write.
// Even if the allocation was done correctly, do we need to write zero at
// the end?

if (fromMem)
// TODO? Check m_evmVersion.hasMcopy() and legalize here or in sol.mcopy
// lowering?
b.create<yul::MCopyOp>(loc, dstDataAddr, srcDataAddr, sizeInBytes);
else
b.create<yul::CallDataCopyOp>(loc, dstDataAddr, srcDataAddr, sizeInBytes);

// Canonicalize the trailing bytes after a variable-length copy.
// This clears the 32-byte word starting at dst + length. As in the
// existing decode helper flow, this write may extend past the
// rounded payload boundary.
Value cleanupAddr = b.create<arith::AddIOp>(loc, dstDataAddr, sizeInBytes);
b.create<yul::MStoreOp>(loc, cleanupAddr, bExt.genI256Const(0));

return dstAddr;
}

Expand Down Expand Up @@ -1796,18 +1832,14 @@ void evm::Builder::genABITupleDecoding(TypeRange tys, Value tupleStart,
Value headAddr = tupleStart;
for (Type ty : tys) {
if (sol::hasDynamicallySizedElt(ty)) {
// TODO: Do we need the "ABI decoding: invalid tuple offset" check here?
Value tailAddr =
b.create<arith::AddIOp>(loc, tupleStart, genLoad(headAddr));

// The `tailAddr` should point to at least 1 32-byte word in the tuple.
// Generate a revert check for that.
auto invalidTailAddrCond = b.create<arith::CmpIOp>(
loc, arith::CmpIPredicate::sge,
b.create<arith::AddIOp>(loc, tailAddr, bExt.genI256Const(31)),
tupleEnd);
genRevertWithMsg(invalidTailAddrCond,
"ABI decoding: invalid calldata array offset", loc);
Value tailOffset = genLoad(headAddr);
auto invalidOffsetCond = b.create<arith::CmpIOp>(
loc, arith::CmpIPredicate::ugt, tailOffset,
bExt.genI256Const(APInt::getLowBitsSet(256, 64)));
genRevertWithMsg(invalidOffsetCond, "ABI decoding: invalid tuple offset",
loc);

Value tailAddr = b.create<arith::AddIOp>(loc, tupleStart, tailOffset);
results.push_back(genABITupleDecoding(ty, tailAddr, fromMem, tupleStart,
tupleEnd, loc));
} else {
Expand Down
Loading