Merge pull request #12050 from ethereum/signedImmutablesBug

Fix signed immutables bug.
This commit is contained in:
chriseth 2021-09-29 11:46:58 +02:00 committed by GitHub
commit 13354c0b9a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 96 additions and 17 deletions

View File

@ -1,10 +1,7 @@
### 0.8.9 (unreleased)
Language Features:
Compiler Features:
Important Bugfixes:
* Immutables: Properly perform sign extension on signed immutables.
Bugfixes:

View File

@ -1,4 +1,14 @@
[
{
"uid": "SOL-2021-3",
"name": "SignedImmutables",
"summary": "Immutable variables of signed integer type shorter than 256 bits can lead to values with invalid higher order bits if inline assembly is used.",
"description": "When immutable variables of signed integer type shorter than 256 bits are read, their higher order bits were unconditionally set to zero. The correct operation would be to sign-extend the value, i.e. set the higher order bits to one if the sign bit is one. This sign-extension is performed by Solidity just prior to when it matters, i.e. when a value is stored in memory, when it is compared or when a division is performed. Because of that, to our knowledge, the only way to access the value in its unclean state is by reading it through inline assembly.",
"link": "https://blog.soliditylang.org/2021/09/29/signed-immutables-bug/",
"introduced": "0.6.5",
"fixed": "0.8.9",
"severity": "very low"
},
{
"uid": "SOL-2021-2",
"name": "ABIDecodeTwoDimensionalArrayMemory",

View File

@ -1333,6 +1333,7 @@
},
"0.6.10": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1342,6 +1343,7 @@
},
"0.6.11": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1351,6 +1353,7 @@
},
"0.6.12": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1402,6 +1405,7 @@
},
"0.6.5": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1415,6 +1419,7 @@
},
"0.6.6": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1427,6 +1432,7 @@
},
"0.6.7": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1439,6 +1445,7 @@
},
"0.6.8": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1448,6 +1455,7 @@
},
"0.6.9": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1458,6 +1466,7 @@
},
"0.7.0": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1467,6 +1476,7 @@
},
"0.7.1": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1477,6 +1487,7 @@
},
"0.7.2": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy",
@ -1486,6 +1497,7 @@
},
"0.7.3": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching",
"EmptyByteArrayCopy"
@ -1494,6 +1506,7 @@
},
"0.7.4": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching"
],
@ -1501,6 +1514,7 @@
},
"0.7.5": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching"
],
@ -1508,6 +1522,7 @@
},
"0.7.6": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching"
],
@ -1515,6 +1530,7 @@
},
"0.8.0": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching"
],
@ -1522,6 +1538,7 @@
},
"0.8.1": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching"
],
@ -1529,6 +1546,7 @@
},
"0.8.2": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory",
"KeccakCaching"
],
@ -1536,28 +1554,39 @@
},
"0.8.3": {
"bugs": [
"SignedImmutables",
"ABIDecodeTwoDimensionalArrayMemory"
],
"released": "2021-03-23"
},
"0.8.4": {
"bugs": [],
"bugs": [
"SignedImmutables"
],
"released": "2021-04-21"
},
"0.8.5": {
"bugs": [],
"bugs": [
"SignedImmutables"
],
"released": "2021-06-10"
},
"0.8.6": {
"bugs": [],
"bugs": [
"SignedImmutables"
],
"released": "2021-06-22"
},
"0.8.7": {
"bugs": [],
"bugs": [
"SignedImmutables"
],
"released": "2021-08-11"
},
"0.8.8": {
"bugs": [],
"bugs": [
"SignedImmutables"
],
"released": "2021-09-27"
}
}

View File

@ -95,9 +95,9 @@ public:
/// @returns the number of bytes consumed in memory.
unsigned loadFromMemory(
unsigned _offset,
Type const& _type = *TypeProvider::uint256(),
bool _fromCalldata = false,
bool _padToWords = false
Type const& _type,
bool _fromCalldata,
bool _padToWords
);
/// Dynamic version of @see loadFromMemory, expects the memory offset on the stack.
/// Stack pre: memory_offset

View File

@ -192,7 +192,12 @@ size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _cont
auto const& immutables = contractType.immutableVariables();
// Push all immutable values on the stack.
for (auto const& immutable: immutables)
CompilerUtils(m_context).loadFromMemory(static_cast<unsigned>(m_context.immutableMemoryOffset(*immutable)), *immutable->annotation().type);
CompilerUtils(m_context).loadFromMemory(
static_cast<unsigned>(m_context.immutableMemoryOffset(*immutable)),
*immutable->annotation().type,
false,
true
);
m_context.pushSubroutineSize(m_context.runtimeSub());
if (immutables.empty())
m_context << Instruction::DUP1;
@ -439,7 +444,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
// retrieve the function signature hash from the calldata
if (!interfaceFunctions.empty())
{
CompilerUtils(m_context).loadFromMemory(0, IntegerType(CompilerUtils::dataStartOffset * 8), true);
CompilerUtils(m_context).loadFromMemory(0, IntegerType(CompilerUtils::dataStartOffset * 8), true, false);
// stack now is: <can-call-non-view-functions>? <funhash>
vector<FixedHash<4>> sortedIDs;

View File

@ -159,7 +159,9 @@ void ImmutableItem::retrieveValue(SourceLocation const&, bool) const
if (m_context.runtimeContext())
CompilerUtils(m_context).loadFromMemory(
static_cast<unsigned>(m_context.immutableMemoryOffset(m_variable)),
*m_dataType
*m_dataType,
false,
true
);
else
for (auto&& slotName: m_context.immutableVariableSlotNames(m_variable))
@ -178,7 +180,7 @@ void ImmutableItem::storeValue(Type const& _sourceType, SourceLocation const&, b
utils.moveIntoStack(m_dataType->sizeOnStack());
else
utils.copyToStackTop(m_dataType->sizeOnStack() + 1, m_dataType->sizeOnStack());
utils.storeInMemoryDynamic(*m_dataType, false);
utils.storeInMemoryDynamic(*m_dataType);
m_context << Instruction::POP;
}

View File

@ -0,0 +1,15 @@
contract C {
int8 immutable a = -2;
bytes2 immutable b = "ab";
function() internal returns (uint) immutable f = g;
function viaasm() view external returns (bytes32 x, bytes32 y) {
int8 _a = a;
bytes2 _b = b;
assembly { x := _a y := _b }
}
function g() internal pure returns (uint) { return 2; }
}
// ====
// compileViaYul: also
// ----
// viaasm() -> 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe, 0x6162000000000000000000000000000000000000000000000000000000000000

View File

@ -0,0 +1,21 @@
contract A {
uint16 public immutable a;
uint16 public immutable b;
uint16 public immutable c;
uint16[3] public x;
constructor() {
c = 0xffff;
b = 0x0f0f;
a = 0x1234;
x = [a, b, c];
}
}
// ====
// compileViaYul: also
// ----
// a() -> 4660
// b() -> 0x0f0f
// c() -> 0xffff
// x(uint256): 0 -> 4660
// x(uint256): 1 -> 0x0f0f
// x(uint256): 2 -> 0xffff