Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLV Reading of Containers often missed a call to ExitContainer #36959

Open
1 of 3 tasks
Alami-Amine opened this issue Jan 4, 2025 · 0 comments
Open
1 of 3 tasks

TLV Reading of Containers often missed a call to ExitContainer #36959

Alami-Amine opened this issue Jan 4, 2025 · 0 comments
Assignees

Comments

@Alami-Amine
Copy link
Contributor

Alami-Amine commented Jan 4, 2025

  • many instances of Parsing TLV Encodings with Containers, a call to ExitContainer() is omitted at the end of Reading that container, this is mostly observed for Top-Level containers at the moment.

  • The API contract of TLVReader::EnterContainer() clearly indicates that this is needed.

  • In addition, not calling TLVReader::ExitContainer() has a side-effect of ALLOWING the successful parsing of a received TLV (container) that is not properly terminated (one that did not call EndContainer()) which should not be allowed.

  • Example: (ExitContainerwas completely missed at the end ) :

    tlvReader.Init(std::move(msg));
    SuccessOrExit(err = tlvReader.Next(containerType, TLV::AnonymousTag()));
    SuccessOrExit(err = tlvReader.EnterContainer(containerType));
    SuccessOrExit(err = tlvReader.Next());
    VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
    SessionResumptionStorage::ResumptionIdStorage resumptionId;
    VerifyOrExit(tlvReader.GetLength() == resumptionId.size(), err = CHIP_ERROR_INVALID_TLV_ELEMENT);
    SuccessOrExit(err = tlvReader.GetBytes(resumptionId.data(), resumptionId.size()));
    SuccessOrExit(err = tlvReader.Next());
    VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
    VerifyOrExit(tlvReader.GetLength() == CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES, err = CHIP_ERROR_INVALID_TLV_ELEMENT);
    SuccessOrExit(err = tlvReader.GetBytes(sigma2ResumeMIC, CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES));
    SuccessOrExit(err = ValidateSigmaResumeMIC(ByteSpan(sigma2ResumeMIC), ByteSpan(mInitiatorRandom), resumptionId,
    ByteSpan(kKDFS2RKeyInfo), ByteSpan(kResume2MIC_Nonce)));
    SuccessOrExit(err = tlvReader.Next());
    VerifyOrExit(TLV::TagNumFromTag(tlvReader.GetTag()) == ++decodeTagIdSeq, err = CHIP_ERROR_INVALID_TLV_TAG);
    SuccessOrExit(err = tlvReader.Get(responderSessionId));
    if (tlvReader.Next() != CHIP_END_OF_TLV)
    {
    SuccessOrExit(err = DecodeMRPParametersIfPresent(TLV::ContextTag(4), tlvReader));
    mExchangeCtxt.Value()->GetSessionHandle()->AsUnauthenticatedSession()->SetRemoteSessionParameters(
    GetRemoteSessionParameters());
    }

  • This was observed in :

    • CASESession.cpp
    • PASESession.cpp
    • PairingSession.cpp ( A corner case only)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant