Remediation of Issues from Filecoin Solidity Audit by Zellic

[snissn]

PR: Remediation of Issues from Filecoin Solidity Audit by Zellic

Overview

This PR implements fixes and improvements based on the security assessment conducted by Zellic for the Filecoin Solidity project. The audit identified seven issues, categorized as one medium, two low, and four informational. All findings have been remediated as detailed in the audit report.

Summary of Changes

The following changes were made to address the audit findings:

1. Replaced assert with if checks + revert using custom errors

   • Eliminates Panic exceptions and ensures proper error messaging.
   • Introduces specific custom errors in Errors.sol:
     • InvalidArrayLength(uint256 expected, uint256 actual)
     • InvalidBooleanType()
     • ExpectedMajorByteString()
     • ExpectedNegativeBigNumTag()
     • ExpectedLowValue27()
   • Fixes Finding: Throwing Panic is not recommended
     Commit 72285939

2. Standardized BigInt deserialization with deserializeBytesBigInt

   • Replaced inconsistent uses of deserializeBigInt with deserializeBytesBigInt across CBOR contracts.
   • Ensured uniform handling of optional BigInt values.
   • Fixes Finding: Discrepancy between BytesBigInt and BigInt is confusing
     Commit 09f2f205
     Commit ff0b3868

3. Fixed buffer allocation type mismatch in serializeChangeWorkerAddressParams

   • Changed buffer size calculations from uint256 to uint64 to prevent truncation issues.
   • Fixes Finding: Buffer allocation type mismatch
     Commit acf7e81e

4. Fixed capacity miscalculation in serializeExtendClaimTermsParams

   • Addressed incorrect CBOR buffer allocation to prevent unnecessary resizing and improve gas efficiency.
   • Fixes Finding: Incorrect CBOR buffer capacity
     Commit 14d60cec

5. Enforced strict array length checks in deserialization

   • Updated deserializeGetDealDataCommitmentReturn to enforce expected array length instead of silently discarding extra entries.
   • Fixes Finding: Extra entries in DealDataCommitmentReturn are silently discarded
     Commit 1eabac47

6. Added validation for BigInt sign byte in deserialization

   • Now rejects invalid sign bytes (only 0x00 or 0x01 allowed).
   • Fixes Finding: The BigInt sign serialization
     Commit 9bf88179

7. Fixed type mismatch in universal receiver params struct

   • Ensured consistent deserialization and serialization of UniversalReceiverParams.type_ as uint32.
   • Fixes Finding: Type mismatch in the universal receiver params struct
     Commit d6bc37bc

Audit Summary

• Audit Report: Filecoin Solidity Audit by Zellic
• Findings: No critical or high-risk issues found. All identified issues have been remediated.

Impact

These changes improve the robustness and security of the Filecoin Solidity library by:

• Enhancing error handling with custom error messages.
• Ensuring correct buffer allocations to prevent potential memory issues.
• Improving consistency in BigInt serialization and deserialization.
• Eliminating silent failures in CBOR decoding.