Sparkn

Summary 📌

Solidity's low-level function call() usage might lead to incorrect event emissions.

Vulnerability Details 🔍

In the process of delegating calls for prize distribution, the _delegate() function eventually gets called, triggering proxy.call(data).

The data in the call may not always pertain to the Distributor.sol contract's distribute() function. For instance, it might eventually call the getConstants() function and still result in a successful call.

This inconsistency would lead the Proxy.sol contract to emit the Distributed(proxy, data) event, even if no actual distribution took place.

Impact 📈

The impact is relatively minimal. However, users can emit misleading events. Ensuring accurate logging of system actions and intentions is important, especially considering SPARKN's targeted adoption by government and public institutions, emphasizing the need for transparency.

Tools Used 🛠️

• Manual audit.

Recommendations 🎯

Two possible fixes are proposed:

• 1️⃣ Introduce a filter to inspect the data parameter's function call. If it deviates from distribute(), the function should revert.

  Implementation Example 💻

  🚧 Note ⚠️: This code is illustrative and has not been tested.

  // 🟢 Getting selector of distribute function

  bytes4 private constant APPROVED_SELECTOR_1 = Distributor.distribute.selector;

  ​

  // 🟢 filterCall() will analyze the data received

  function filterCall(bytes calldata data) external {

  // 🟢 extractSelector() is defined below

  bytes4 selector = extractSelector(data);

  ​

  require(

  selector == APPROVED_SELECTOR_1,

  "Function not approved"

  );

  ​

  // 🟢 If the selector is approved, perform the call

  proxy.call(data);

  // rest of code...

  }

  ​

  function extractSelector(bytes memory data) public pure returns (bytes4) {

  require(data.length >= 4, "Data too short");

  bytes4 selector;

  // solhint-disable-next-line no-inline-assembly

  assembly {

  selector := mload(add(data, 0x20))

  }

  return selector;

  }

• 2️⃣ Examine the return data. If distribute() gets invoked, the output will be 0; otherwise, the function should revert.

  Implementation Example 💻

  🚧 Note ⚠️: This code is illustrative and has not been tested.

  function _distribute(address proxy, bytes calldata data) internal {

  ​

  // 🟢 Notice we are checking the returned output just in case

  (bool success, bytes memory result) = proxy.call(data);

  require(result.length == 0, "Data was returned, distribute was not called, reverting.");

  ​

  if (!success) revert ProxyFactory__DelegateCallFailed();

  emit Distributed(proxy, data);

  }

Given its straightforwardness and the absence of any other function in Distributor.sol returning nothing, solution 2️⃣ stands out as the preferable choice.

If in the future the Distributor.sol gets more complex and has more functions returning something, implementation 1️⃣ should be considered.