Disallow delete on types containing nested mappings.

Author: Marenz

Changelog.md

@@ -1,8 +1,10 @@
 ### 0.9.0 (unreleased)
 
 Breaking changes:
- * `error` is now a keyword that can only be used for defining errors.
+ * 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.
+ * ``error`` is now a keyword that can only be used for defining errors.
 
 
 ### 0.8.8 (unreleased)

docs/security-considerations.rst

@@ -284,48 +284,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
-same happens, 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
-``mapping`` is also ignored in assignments of structs or arrays containing a
+or popping the array is not possible.
+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.
+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

@@ -1613,6 +1613,13 @@ bool TypeChecker::visit(UnaryOperation const& _operation)
 	_operation.annotation().isPure = !modifying && *_operation.subExpression().annotation().isPure;
 	_operation.annotation().isLValue = false;
 
+	if (op == Token::Delete && subExprType->nameable() && subExprType->containsNestedMapping())
+		m_errorReporter.fatalTypeError(
+			2811_error,
+			_operation.location(),
+			"Type containing a (nested) mapping cannot be deleted."
+		);
+
 	return false;
 }
 
@@ -2839,6 +2846,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()

test/libsolidity/semanticTests/storage/mappings_array2d_pop_delete.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,20 +25,10 @@ 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
 // ----
 // 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/semanticTests/storage/mappings_array_pop_delete.sol

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

test/libsolidity/semanticTests/structs/delete_struct.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/array/pop/storage_with_mapping_pop.sol

@@ -0,0 +1,14 @@
+contract D {
+	struct Test {
+		mapping(address => uint) balances;
+	}
+
+	Test test;
+
+	constructor()
+	{
+		delete test;
+	}
+}
+// ----
+// TypeError 2811: (102-113): Type containing a (nested) mapping cannot be deleted.