Datachain security record

`_deltaSourceUpdate()` is designed to apply the `deltaCredit` to replenish the soft partitions of the liquidity pool. The `diff` between the target total balance and current balance (the deficit) is calculated for each pool. `deltaCredit` is then applied to replenish each pool according to their weights. However, if `fullMode == true` and `totalDiff <= deltaCredit`, the `diff[i]` is never applied to `amtToCredit`. In this scenario, only the `excessCredit` is distributed to the pools. The inline and NatSpec comments further validate that this is a missing requirement.

The expected value of arbitraging a price drift between two pegged assets is equal to: ```js expectedValue = (dstPrice - srcPrice) * amountGD ``` The protocol drift fee is equal to: ```js (amountGd * srcPrice) / dstPrice ``` The current calculation far overcharges the protocol drift fee.

`IBCChannelHandshake.channelOpenAck()` does not confirm that the connection is open before opening the channel. As a result, an ibc-solidity chain can open a channel although the connection in which it resides is not open. This is a fundamental flaw as the connection handshake must be complete before any packets can be sent or received through it to ensure that two ConnectionEnds between chains are totally in sync. Additionally, creating a channel on a connection that hasn't completed the handshake will totally bypass all logic contained in `IBCConnection.connectionOpenConfirm()`. Another impact is that if the channel were created in this fashion, from the ChannelEnd on ChainB, `channelCloseInit()` could never be invoked, as this function requires that the `ConnectionEnd.State == STATE_OPEN`, which could result in the ChainB ChannelEnd being forced to continously receive what could be malicious packets from a counterparty. Throughout this implementation there is a missing enforcement of an open connection as defined in the specs as the Datachain team incorrectly assumes that an open channel implies an open connection. The comment `// NOTE: We can assume here that the connection state is OPEN because the channel state is OPEN` notes all places in the codebase where this assumption is made. Resolving this issue will allow this assumption to hold true, and we have not identified such a clear exploit resulting from the missing checks, however, we encourage the Datachain team to take a closer look here.

In the function `IBCChannelPacketTimeout.timeoutPacket()`, when closing an ordered channel due to packet timeout, the closed channel state is not written to the provable store. This creates the possibility for counterparty chains to be able to verify a different state of this channel that what is intended.

During several packet operations there is missing logic in the ibc-solidity implementation when the Channel is in the `STATE_FLUSHING` state and actively completing an upgrade. According to the specs, this logic enforces a counterparty upgrade timeout. The logic is missing from the `acknowledgePacket()` and `timeoutPacket()` functions.

`IBCClient.updateClientCommitments()` does not enforce that the `commitments` mapping is zero for the `consensusStateCommitmentKey` that is trying to be written. This is directly contradictory to what is documented in ADR-001 by the Datachain team. Perhaps, if the key exists, and is equivalent to the consensus state we derived from the light client, it would make sense to continue. However, the existence of the key with a different consensus state should signal that there is something out of sync between the clients. For example, in the case of a reorg, there could be a consensus state written at that key that is from a malicious series of blocks, that can be accepted as valid state alongside valid blocks.

The two functions `timeoutPacket()` and `timeoutOnClose()` first check if a packet's timeout has been exceeded, and if so, update the channel's state accordingly. In order to avoid replaying the same timeout multiple times, the packet's commitment key can be removed after use, as is suggested for both functions in the [ICS-04 specification](https://github.com/cosmos/ibc/blob/47fea20d4d400e967721396092c6b43398c65d78/spec/core/ics-004-channel-and-packet-semantics/README.md). However, this check does not occur in `timeoutOnClose()`. Note that if `timeoutOnClose()` is called multiple times for the same packet, the following module call will occur each time: ``` lookupModuleByChannel(msg_.packet.sourcePort, msg_.packet.sourceChannel).onTimeoutPacket( msg_.packet, _msgSender() ``` Since we cannot know how each different module will handle this call, this replay scenario should be avoided. This could be particularly problematic if the module refunds a user's tokens in this scenario.

In the `check_tdx_module_tcb_status` function, a TDX module identity ID is generated using lowercase hexadecimal formatting: ```rust let tdx_module_identity_id = format!("TDX_{:02x}", tdx_module_version); ``` This identifier is then used in a direct string comparison when filtering module identities: ```rust .filter(|m| m.id == tdx_module_identity_id) ``` The reference C++ implementation in Intel's SGX-TDX-DCAP-QuoteVerificationLibrary explicitly normalizes string case before comparison: ```cpp std::string id = tdxModuleIdentity.getId(); std::transform(id.begin(), id.end(), id.begin(), ::toupper); return (id == tdxModuleIdentityId); ``` If a mismatch occurs due to case sensitivity: - The function will not find a matching TDX module identity - It will return `TdxModuleTcbValidationStatus::TcbNotSupported` - This will likely cause legitimate TDX attestations to fail verification While this is not a security vulnerability allowing unauthorized access, it is a correctness issue that could impact system functionality by causing false rejections of valid attestations.

When a failed transaction can no longer be retried, a permanent difference between the last known balance on the destination chain and the balance on the source chain grows by the value of the failed transaction, such that: ```js chainA.peerPoolInfo[chainB].lastKnownBalance >= chainB.peerPoolInfo[chainA].balance + totalFailedTransfers ``` The last known balance is used to determine how much a pool needs to be replenished in the delta algorithm. When the last known balance is greater than the actual balance available, the pool is underestimating how much liquidity is needed to replenish the pool. This gap will grow larger as the number of locked funds due to failed crosschain transactions increase over time. While pools can be rebalanced to accomodate for bandwidth requirements, many of the fees depend on accurately assessing pool health. A pool that seems to be closer to its target balance than it is in actuality cannot accurately manage incentives through fees.

The `BridgeFallback` contract uses the `onlySelf` modifier to restrict access to its `addRevert*()` functions, intending for these functions to be called only internally within the `BridgeFallback` contract itself. However, the `retryOnReceive` function, designed to retry failed cross-chain operations, attempts to call these `addRevert*()` functions when a retry fails, using a `try-catch` block. In that case, the `msg.sender` will be the external caller of `retryOnReceive`, not the `BridgeFallback` contract itself. This causes a revert, because the `onlySelf` modifier evaluates as false.