TL;DR: We have reviewed the implementation of Diamond’s proposed standard contract proposal for quality improvement and cannot recommend it in its current form – but see our recommendations and guide to the quality improvement strategy for contracts.
We recently revised the implementation of the Standard Diamond Code, a new model of scalability. It is worthwhile, but the Diamond proposal and its implementation raise many questions. The code has been reworked, with many unnecessary complications, and we cannot recommend it at this time.
Of course, the proposal remains a project with opportunities for growth and improvement. The work standard for extensibility must
- Clear and simple execution. Standards should be easily readable to facilitate integration with third party applications.
- Detailed checklist of upgrade procedures. Modernisation is a risky process that needs to be explained in detail.
- Reduce the impact of common upgrade errors, including shadows and collisions. Although some errors are easy to detect, they can lead to serious problems. Note that many pitfalls that can be mitigated can be tested to slip.
- A list of the risks involved. Modernisation is difficult; it can hide safety problems or suggest that the risks are insignificant. TRMs are suggestions for improving Ethereum, not for commercial advertising.
- The tests are integrated in the most common test platforms. The tests must show how the system is used, how a new implementation is updated and how an update can fail.
Unfortunately, Mr Diamond’s proposal does not take these points into account. This is a pity, because we would like to see an updated standard that solves or at least mitigates the main safety pitfalls of updated contracts. In fact, the authors of standards should assume that developers will make mistakes, and should strive for a standard that facilitates those mistakes.
Still, there is a lot to learn from the diamond stock. Read more:
- How does the Diamond offer work?
- That’s what our investigation uncovered.
- Our recommendations
- Proven standard procedures for modernisation
The Diamond Stock Paradigma
Proposal Diamond is an unfinished project as defined in EIP 2535. The draft states that a new paradigm of contract modernisation is proposed on the basis of delegation of powers. (For your information, here is an overview of how scalability works) The PIE 2535 offers the use of
- Implementation summary table
- Random memory pointer
Summary table
The upgrade option based on call delegation works mainly with two components – proxy and implementation:
.
Figure 1 : Possibility of updating on the basis of a delegation on request with a single implementation
The user communicates with the proxy server and the proxy server delegates the implementation call. The implementation code is executed and the storage is stored in the proxy server.
Using the lookup table, you can delegate release orders to multiple contract runs, selecting the appropriate run based on the function performed:
Figure 2 : Delegate on-demand updates with multiple implementations.
This scheme is not new; other projects in the past used such look-up tables to allow for updates. See the example under ColonyNetwork.
Random memory pointer
The proposal also suggests the use of a recently introduced function in Solidity: a pointer to each repository, which (as the name suggests) makes it possible to assign a pointer to the repository at any location.
Since the storage is stored on a proxy server, the implemented storage schedule must match the storage schedule of the proxy server. It can be difficult to reproduce this layout during updates (see examples here).
The DPI suggests that each implementation should have a corresponding structure for storing implementation variables and a pointer to each location where the structure is stored. This function is similar to an unstructured memory model, where the new Strength function allows you to use the structure instead of a single variable.
It is assumed that two structures of two different implementations cannot collide with each other as long as their basic orientations are different.
bytes32 Constant POSITION = keccak256 ( some_string ); struct MyStruct { uint var1; uint var2; } Function get_struct() internal net return(MyStruct memory ds) { bytes32 Position = POSITION; assembly { ds_slot := position } } }.
Figure 3 : An example of a memory pointer.
Figure 4 : Presentation of the memory index.
VAT, what is a diamond?
EPIP 2535 introduces diamond terminology, where the word diamond means a reliable contract, facet for the execution, etc. It is not clear why this terminology was introduced, especially since the standard modernization terminology is well known and defined. Here is the key that will help you translate the sentence as you get through it:
| Diamond dictionary | Common name |
| Diamond | power of attorney for |
| Faiset | the implementation of |
| Cutting | Update |
| Lupa | List of delegated functions |
| Reserve diamond | Non-extendable |
| Single diamond | Deleting the update functions |
Figure 5 : In the diamond proposal new terms are used to refer to existing ideas.
Audit findings and recommendations
Our evaluation of diamond sales revealed this:
- The code is overloaded and contains several failed optimizations
- The use of reminders carries risks
- There was a shadow function in the code base…
- There is no existence check in the contract.
- The diamond dictionary adds unnecessary complexity
Code added
Although the model proposed in the IPR is simple, its actual implementation is difficult to read and monitor, which increases the likelihood of problems.
For example, most of the data stored in a chain is voluminous. While a record only needs one lookup table, from the function signature to the implementation address, the EIP identifies numerous interfaces that require additional data storage:
IDiamondLoupe { //// These functions must often be called by /// tools. Structure facet [Address facet address; Bytes4[] Function selections; } /// @melding Gets all facet addresses and their four-byte function selectors. /// @Return Facets_ The facets() function returns the appearance (facet[] facets_); //// @Comment Returns all functions supported by specific facets. /// @param _facet Address Facet. /// @ return facetFunctionSelectors_functionSelectors(address _facet) Returns the appearance (bytes4[] facet memoryFunctionSelectors_); //// @notice Returns all facet addresses used by a facet. /// @ return facetAddresses_ The function facetAddresses() returns the appearance (address[] facet memoryAddresses_); //// @notice Gets the edge this selector supports /// @dev If the facet is not found, return the address(0). /// @param _functionSelector Selects functions. /// @returnfacetteAddress_ facet address. The function facetAddress(bytes4 _functionSelector) returns the appearance (facetAddress_ address);
Figure 6 : Diamond interfaces.
Here facetFunctionSelectors returns all selectors for the implementation functions. This information will only be useful for those elements outside the chain that can already extract information from the events of the contract. Such a function is not necessary in a string, mainly because it increases the complexity of the code considerably.
Moreover, a large part of the complexity of the code is due to optimization in places where it is not necessary. For example, the function used to update the implementation should be simple. When it accepts a new address and a new signature, it must update the corresponding entry in the lookup table. Well, part of that job is this:
// Add or replace functions as (newFacet != 0) { // Add and replace selectors for (uint selectorIndex; selectorIndex < numSelectors; selectorIndex++) { bytes4 selector; assembly { selector := mload(add(facetCut,position)) } Position += 4; bytes32 oldFacet = ds.facets [selector]; // add as (oldFacet == 0) { // update the last slot at the end of slot.updateLastSlot = true; ds.facets [selector] = newFacet | bytes32(selectorSlotLength) <lt;64 | bytes32(selectorSlotsLength); // remove the position of the selector in the slot and add the selector slot.selectorSlot = lock.selectorSlot and ~(CLEAR_SELECTOR_MASK >> selectorSlotLength * 32) | bytes32(selector) >> selectorSlotLength * 32; selectorSlotLength++; // if the lock is full, write it into memory as(selectorSlotLength == 8) { ds.selectorSlotsSlots [selectorSlotsLength] = lock.selectorSlot; slot.selectorSlot = 0; selectorSlotLength = 0; selectorSlotsLength++; } } // replace another { required(bytes20(oldFacet) != bytes20(newFacet), function cut to the same facet); // replace the address of the old facet ds.facets [selector] = oldFacet ; CLEAR_ADDRESS_MASK | newFacet ; } } }.
Figure 7 : The update function.
Great efforts have been made to optimise the gas performance of this function. But contract modernization is rarely done, so it will never be an expensive operation, regardless of the price of the gas.
In another example of unnecessary complexity, bit operations are used instead of the :
uint selectorSlotsLength = uint128(slot.originalSelectorSlotsLength); uint selectorSlotsLength = uint128(slot.originalSelectorSlotsLength >> 128); // uint32 selectorSlotsLength, uint32 selectorSlotsLength // selectorSlotsLength – Number of 32-byte slots in SelectorSlots. // selectorSlotsLength – number of selectors in the last slot // selectorSlotsLength – number of selectors in the last slot // selectorSlots ;
Figure 8 : Use bit operations instead of structure.
Our recommendations:
- Always look for simplicity and save as many string codes as possible.
- When writing a new standard, make sure that the code is readable and easy to understand.
- Analyze your needs before performing optimizations.
Memory pointer risks
Despite the claim that collisions are impossible if the base points are different, a malicious contract can hit a variable from another implementation. In principle this is possible because Solidity stores variables and affects the display or arrays. For example:
TestCollision{ // The contract is a contract with two implementations, A and B // A has an interleaved structure // A and B have different underlying memory points // But writing to B leads to a conflict between the underlying memory pointer B // and A.ds.my_items[0].elems bytes32 of the public constant A_STORAGE = keccak256( A ); struct InnerStructure{ uint[] elems; }. struct St_A { InnerStructure[] my_items; }. The function pointer_to_A() internal net returns(St_A store s) { bytes32 position = A_STORAGE; assembly { s_slot := position } }. Bytes32 of the public constant B_STORAGE = keccak256 ( hex78c8663007d5434a0acd246a3c741b54aecf2fefff4284f2d3604b72f2649114 ) ; Structure St_B { uint trap ; } pointer_to_B() function return internal network (St_B Storage) { bytes32 Position = B_STORAGE ; Board { s_slot := position } } constructor() public{ St_A storage ds = pointer_to_A(); ds.my_items.push(); ds.my_items[0].elems.push(100); } Function get_balance() View public(uint){ St_A memory ds = pointer_to_A(); return ds.my_items[0].elems[0]; } Function exploit(uint new_val) public{ St_B memory ds = pointer_to_B(); ds.val = new_val; }. }
Figure 9 : Colliding memory points.
When in use, the basic pointer B_STORAGE is actually written to my_items[0].elems[0], which is read from the basic pointer A_STORAGE. A malicious owner can push an update that seems benign but contains a backdoor.
The CRT has no recommendations to prevent these malicious collisions. If the pointer is reused after disposal, the re-use will also endanger the data.
Our recommendations
- Manipulating low-level memory is risky, so be very careful when designing a system that depends on it.
- The use of unstructured storage with modernization structures is an interesting idea, but it requires careful documentation and advice, which should be checked in the basic index.
Shadow function
Contracts that need to be updated often contain functions in the proxy department that are followed by functions that need to be delegated. Calls to these functions are never delegated because they are executed in a proxy server. Moreover, the corresponding code is not updated.
Agent contract { constructor(…) public{ // my_targeted_function() // add as delegated function } function my_targeted_function() public{ } fallback() public{ // delegate implementation }}}.
Figure 10 : Simplification of the shadow problem.
Although this problem is known and the code was revised by the author of the CFM, we found two cases of shadow functions in contracts.
Our recommendations
- When writing an upgradeable contract, use crytic.io or slither-check upgradeability to capture shadows.
- This issue highlights an important point: Developers make mistakes. Any new standard must include the reduction of common errors if it is to work better than custom solutions.
No contract review
Another common error is the lack of revision of the contract code. If the agent has delegated an incorrect address or destroyed the implementation, the call for this implementation will be successful again, even if the code has not been executed (see the Solidity Documentation). As a result, the calling subscriber will not notice the problem and it is likely that this behaviour will violate the contract with a third party.
fallback() external pay { DiamondStorage storage ds; bytes32 position = DiamondStorageContract.DIAMOND_STORAGE_POSITION; assembly { ds_slot := position } address facet = address(bytes20(ds.facets[msg.sig])); require(facet != address(0), the function is not available.); assembly { calldatacopy(0, 0, calldatasize()) leaves result := delegatecall(gas(), facet, 0, calldatasize(), 0, 0) leaves size := returnatacopy(0, 0, size) returnatacopy(0, 0, size) switch result case 0 {revert(0, size)} default {return (0, size)}.
Figure 11 : The backtracking function without checking the existence of the contract.
Our recommendations
- Always search for a contract when you call for any contract.
- Carry out this gas cost check only if the call does not return any data, because the reverse result means that a code has been executed.
Dictionary of Unnecessary Diamonds
As mentioned above, Diamond’s proposal relies heavily on a newly created vocabulary. It is prone to errors, complicates the revision process and does not benefit the developers.
- A diamond is a contract that uses the functions of facets to make function calls. A diamond can have one or more facets.
- The word facet comes from the diamond industry. It is the side or flat surface of a diamond. A diamond can have many facets. In this standard, a cut is referred to as a contract with one or more functions that perform the function of a diamond.
- A magnifying glass is a magnifying glass that allows you to see the diamonds. In this standard, the magnifying glass is cut, which has the function of looking at the diamond and its facets.
Figure 12 : CIP redefines standard provisions that have nothing to do with software engineering.
Our recommendation
- Use a common and familiar dictionary and don’t invent terminology if you don’t need it.
Thediamond proposal is a dead end?
As mentioned above, we still believe that the community would benefit from a standardized modernization program. However, the current supply of diamonds does not meet the expected security requirements and does not offer sufficient advantages over bespoke sales.
However, this proposal remains a project and can be converted into something simpler and better. Even if this is not the case, some of the existing methods, such as dependency tables and random memory points, deserve further investigation.
Well… Is scalability possible or not?
Over the years, we have reviewed many evolving contracts and published various analyses on the subject. Updating is complex, error-prone and increases risk, but we still do not recommend it as a solution. But developers who need upgrades should include them in their contracts:
And don’t hesitate to contact us if you have any questions about your renewal strategy. We’re here to help you!
*** This is a syndicated network of security bloggers from the Trail of Bits blog, written by josselinfeist. The original message can be found at the following address: https://blog.trailofbits.com/2020/10/30/good-idea-bad-design-how-the-diamond-standard-falls-short/.
Related Tags:
hifive unmatched price,sifive,risc-v linux board,lofive risc-v,sipeed maixduino kit for risc-v ai + iot,sifive freedom e310,risc-v raspberry pi,polarfire soc icicle kit