Wrong implement of `AutomataDcapV3Attestation._attestationTcbIsValid` function

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-03-taiko/blob/main/packages/protocol/contracts/automata-attestation/AutomataDcapV3Attestation.sol#L126-#L136

Vulnerability details

Vulnerability details

AutomataDcapV3Attestation._attestationTcbIsValid() function is implemented as below:

function _attestationTcbIsValid(TCBInfoStruct.TCBStatus status)
    internal
    pure
    virtual
    returns (bool valid)
{
    return status == TCBInfoStruct.TCBStatus.OK
        || status == TCBInfoStruct.TCBStatus.TCB_SW_HARDENING_NEEDED
        || status == TCBInfoStruct.TCBStatus.TCB_CONFIGURATION_AND_SW_HARDENING_NEEDED
        || status == TCBInfoStruct.TCBStatus.TCB_OUT_OF_DATE_CONFIGURATION_NEEDED;
}

But compare to original contract , it does not have this condition with status variable: || status == TCBInfoStruct.TCBStatus.TCB_OUT_OF_DATE_CONFIGURATION_NEEDED

function _attestationTcbIsValid(TCBInfoStruct.TCBStatus status) internal pure virtual returns (bool valid) {
    return status == TCBInfoStruct.TCBStatus.OK || status == TCBInfoStruct.TCBStatus.TCB_SW_HARDENING_NEEDED
        || status == TCBInfoStruct.TCBStatus.TCB_CONFIGURATION_AND_SW_HARDENING_NEEDED;
}

By implement wrongly, it can lead to unexpected result

Impact

Unexpected result can be happened due to forking original contract wrongly

Tools Used

Manual review

Recommended Mitigation Steps

Update _attestationTcbIsValid function same as original contract

Assessed type

Context

[c4-pre-sort]

minhquanym marked the issue as sufficient quality report

[smtmfft]

This is kind of a loosen requirement (recover from error) for correct sgx quote (proof) but out of date configurations(usually BIOS). The recovered error is actually this one:

From https://github.com/intel/SGXDataCenterAttestationPrimitives/blob/cd27223301e7c2bc80c9c5084ad6f5c2b9d24f5c/QuoteVerification/QvE/Include/sgx_qve_header.h#L51

   SGX_QL_QV_RESULT_OUT_OF_DATE_CONFIG_NEEDED = SGX_QL_QV_MK_ERROR(0x0003), ///< The Quote is good but the TCB level of the platform is out of
                                                                            ///< date and additional configuration of the SGX Platform at its
                                                                            ///< current patching level may be needed. The platform needs
                                                                            ///< patching to be at the latest TCB level

Which means the quote generation is ok, but the TCB needs updates (to sync with intel). However, the SGX machines do normally not catch up with the latest intel updates, especially for cloud servers, it takes months to stablize the updates. so here we loosen the check. As long as the quote is ok, no real security concern here based on my understanding.

[c4-sponsor]

dantaik (sponsor) acknowledged

[c4-sponsor]

dantaik marked the issue as disagree with severity

[0xean]

warden shows no impact, QA

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[c4-judge]

0xean marked the issue as grade-b

[deeplake31337]

From latest intel gdx documentation link: SGX_QL_QV_RESULT_OUT_OF_DATE_CONFIG_NEEDED - The SGX platform firmware and SW are not at the latest security patching level. The platform needs to be patched with firmware and/or software patches. There are also platform hardware configurations that may expose the enclave to vulnerabilities. These configuration vulnerabilities can be mitigated with the appropriate platform configuration changes. Applying both the updated patches and the appropriate platform configuration changes will produce an SGX_QL_QV_RESULT_OK verification result. - page 41

Which mean that security concern is yes at here, it is necessary for SGX to be upgraded to latest version. So i think this issue should be marked as medium at least

[dantaik]

Thank you for sharing the info. Marking it medium works for me.

[0xean]

leaving as judged, introducing new information during PJQA isn't fair to other wardens.

[deeplake31337]

i dont think it is introducing new information, just to explain the unexpected result that can happen in the report. In the original report, i showed that it is forked wrongly and can lead to unexpected result, PJQA is just to explain deeper how it can be. Also sponsor @dantaik agreed with medium severity.

[deeplake31337]

also it is somewhat like prove proof in the PJQA, so i dont think it isn't fair to other wardens

[0xean]

this is final.