Disallow delete on types containing nested mappings.

Changelog.md

@@ -1,6 +1,8 @@
 ### 0.9.0 (unreleased)
 
 Breaking changes:
+ * Disallow ``.pop()`` on arrays containing nested mappings.
+ * Disallow ``delete`` on types that contain nested mappings.
  * Inline Assembly: Consider functions, function parameters and return variables for shadowing checks.
 
 

docs/security-considerations.rst

@@ -317,48 +317,12 @@ key-value data structure that does not keep track of the keys that were
 assigned a non-zero value.  Because of that, cleaning a mapping without extra
 information about the written keys is not possible.
 If a ``mapping`` is used as the base type of a dynamic storage array, deleting
-or popping the array will have no effect over the ``mapping`` elements.  The
+or popping the array is not possible.
-same happens, for example, if a ``mapping`` is used as the type of a member
+The same is true, for example, if a ``mapping`` is used as the type of a member
-field of a ``struct`` that is the base type of a dynamic storage array.  The
+field of a ``struct`` that is the base type of a dynamic storage array.
-``mapping`` is also ignored in assignments of structs or arrays containing a
+It is also not possible to assign new values to structs or arrays containing a
 ``mapping``.
 
-.. code-block:: solidity
-
-    // SPDX-License-Identifier: GPL-3.0
-    pragma solidity >=0.6.0 <0.9.0;
-
-    contract Map {
-        mapping (uint => uint)[] array;
-
-        function allocate(uint _newMaps) public {
-            for (uint i = 0; i < _newMaps; i++)
-                array.push();
-        }
-
-        function writeMap(uint _map, uint _key, uint _value) public {
-            array[_map][_key] = _value;
-        }
-
-        function readMap(uint _map, uint _key) public view returns (uint) {
-            return array[_map][_key];
-        }
-
-        function eraseMaps() public {
-            delete array;
-        }
-    }
-
-Consider the example above and the following sequence of calls: ``allocate(10)``,
-``writeMap(4, 128, 256)``.
-At this point, calling ``readMap(4, 128)`` returns 256.
-If we call ``eraseMaps``, the length of state variable ``array`` is zeroed, but
-since its ``mapping`` elements cannot be zeroed, their information stays alive
-in the contract's storage.
-After deleting ``array``, calling ``allocate(5)`` allows us to access
-``array[4]`` again, and calling ``readMap(4, 128)`` returns 256 even without
-another call to ``writeMap``.
-
 If your ``mapping`` information must be deleted, consider using a library similar to
 `iterable mapping <https://github.com/ethereum/dapp-bin/blob/master/library/iterable_mapping.sol>`_,
 allowing you to traverse the keys and delete their values in the appropriate ``mapping``.

libsolidity/analysis/TypeChecker.cpp

@@ -1594,11 +1594,18 @@ bool TypeChecker::visit(UnaryOperation const& _operation)
 		requireLValue(_operation.subExpression(), false);
 	else
 		_operation.subExpression().accept(*this);
+
 	Type const* subExprType = type(_operation.subExpression());
-	Type const* t = type(_operation.subExpression())->unaryOperatorResult(op);
+	TypeResult result = subExprType->unaryOperatorResult(op);
-	if (!t)
+	Type const* t = result;
+	if (!result)
 	{
-		string description = "Unary operator " + string(TokenTraits::toString(op)) + " cannot be applied to type " + subExprType->toString();
+		string description = "Unary operator " +
+			string(TokenTraits::toString(op)) +
+			" cannot be applied to type " +
+			subExprType->toString() +
+			(result.message().empty() ? "" : (": " + result.message()));
+
 		if (modifying)
 			// Cannot just report the error, ignore the unary operator, and continue,
 			// because the sub-expression was already processed with requireLValue()
@@ -2838,6 +2845,15 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess)
 			);
 
 		if (
+			funType->kind() == FunctionType::Kind::ArrayPop &&
+			exprType->containsNestedMapping()
+		)
+			m_errorReporter.typeError(
+				6298_error,
+				_memberAccess.location(),
+				"Storage arrays with nested mappings do not support .pop()."
+			);
+		else if (
 			funType->kind() == FunctionType::Kind::ArrayPush &&
 			arguments.value().numArguments() != 0 &&
 			exprType->containsNestedMapping()

libsolidity/ast/Types.cpp

@@ -1449,6 +1449,10 @@ TypeResult ReferenceType::unaryOperatorResult(Token _operator) const
 {
 	if (_operator != Token::Delete)
 		return nullptr;
+
+	if (containsNestedMapping())
+		return TypeResult::err("Contains a (possibly nested) mapping");
+
 	// delete can be used on everything except calldata references or storage pointers
 	// (storage references are ok)
 	switch (location())

test/libsolidity/semanticTests/storage/mappings_array2d_pop_delete.sol

@@ -1,41 +0,0 @@
-contract C {
-	mapping (uint => uint)[][] a;
-
-	function n1(uint key, uint value) public {
-		a.push();
-		mapping (uint => uint)[] storage b = a[a.length - 1];
-		b.push();
-		b[b.length - 1][key] = value;
-	}
-
-	function n2() public {
-		a.push();
-		mapping (uint => uint)[] storage b = a[a.length - 1];
-		b.push();
-	}
-
-	function map(uint key) public view returns (uint) {
-		mapping (uint => uint)[] storage b = a[a.length - 1];
-		return b[b.length - 1][key];
-	}
-
-	function p() public {
-		a.pop();
-	}
-
-	function d() public returns (uint) {
-		delete a;
-		return a.length;
-	}
-}
-// ====
-// compileViaYul: also
-// ----
-// n1(uint256,uint256): 42, 64 ->
-// map(uint256): 42 -> 64
-// p() ->
-// n2() ->
-// map(uint256): 42 -> 64
-// d() -> 0
-// n2() ->
-// map(uint256): 42 -> 64

test/libsolidity/semanticTests/storage/mappings_array_pop_delete.sol

@@ -1,36 +0,0 @@
-contract C {
-	mapping (uint => uint)[] a;
-
-	function n1(uint key, uint value) public {
-		a.push();
-		a[a.length - 1][key] = value;
-	}
-
-	function n2() public {
-		a.push();
-	}
-
-	function map(uint key) public view returns (uint) {
-		return a[a.length - 1][key];
-	}
-
-	function p() public {
-		a.pop();
-	}
-
-	function d() public returns (uint) {
-		delete a;
-		return a.length;
-	}
-}
-// ====
-// compileViaYul: also
-// ----
-// n1(uint256,uint256): 42, 64 ->
-// map(uint256): 42 -> 64
-// p() ->
-// n2() ->
-// map(uint256): 42 -> 64
-// d() -> 0
-// n2() ->
-// map(uint256): 42 -> 64

test/libsolidity/semanticTests/structs/delete_struct.sol

@@ -2,23 +2,17 @@ contract test {
     struct topStruct {
         nestedStruct nstr;
         uint topValue;
-        mapping (uint => uint) topMapping;
     }
     uint toDelete;
     topStruct str;
     struct nestedStruct {
         uint nestedValue;
-        mapping (uint => bool) nestedMapping;
     }
     constructor() {
         toDelete = 5;
         str.topValue = 1;
-        str.topMapping[0] = 1;
-        str.topMapping[1] = 2;
 
         str.nstr.nestedValue = 2;
-        str.nstr.nestedMapping[0] = true;
-        str.nstr.nestedMapping[1] = false;
         delete str;
         delete toDelete;
     }
@@ -31,12 +25,6 @@ contract test {
     function getNestedValue() public returns(uint nestedValue){
         nestedValue = str.nstr.nestedValue;
     }
-    function getTopMapping(uint index) public returns(uint ret) {
-        ret = str.topMapping[index];
-    }
-    function getNestedMapping(uint index) public returns(bool ret) {
-        return str.nstr.nestedMapping[index];
-    }
 }
 // ====
 // compileViaYul: also
@@ -44,7 +32,3 @@ contract test {
 // getToDelete() -> 0
 // getTopValue() -> 0
 // getNestedValue() -> 0 #mapping values should be the same#
-// getTopMapping(uint256): 0 -> 1
-// getTopMapping(uint256): 1 -> 2
-// getNestedMapping(uint256): 0 -> true
-// getNestedMapping(uint256): 1 -> false

test/libsolidity/syntaxTests/array/pop/storage_with_mapping_pop.sol

@@ -6,3 +6,4 @@ contract C {
     }
 }
 // ----
+// TypeError 6298: (109-118): Storage arrays with nested mappings do not support .pop().

test/libsolidity/syntaxTests/negation.sol

@@ -6,4 +6,4 @@ contract test {
     }
 }
 // ----
-// TypeError 4907: (97-99): Unary operator - cannot be applied to type uint256
+// TypeError 4907: (97-99): Unary operator - cannot be applied to type uint256: Unary negation is only allowed for signed integers.

test/libsolidity/syntaxTests/types/mapping/assign_storage_struct_with_mapping.sol

@@ -0,0 +1,15 @@
+contract D {
+	struct Test {
+		mapping(address => uint) balances;
+	}
+
+	Test test;
+
+	constructor()
+	{
+		test = Test();
+	}
+}
+// ----
+// TypeError 9214: (102-106): Types in storage containing (nested) mappings cannot be assigned to.
+// TypeError 9515: (109-115): Struct containing a (nested) mapping cannot be constructed.

test/libsolidity/syntaxTests/types/mapping/delete_storage_struct_with_mapping.sol

@@ -0,0 +1,14 @@
+contract D {
+	struct Test {
+		mapping(address => uint) balances;
+	}
+
+	Test test;
+
+	constructor()
+	{
+		delete test;
+	}
+}
+// ----
+// TypeError 9767: (102-113): Unary operator delete cannot be applied to type struct D.Test storage ref: Contains a (possibly nested) mapping