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

[BUG] Heap-Use-After-Free in CASESession during TCP Connection Termination #36732

Closed
BoB13-Matter opened this issue Dec 5, 2024 · 0 comments · Fixed by #36879
Closed

[BUG] Heap-Use-After-Free in CASESession during TCP Connection Termination #36732

BoB13-Matter opened this issue Dec 5, 2024 · 0 comments · Fixed by #36879
Assignees
Labels
bug Something isn't working needs triage

Comments

@BoB13-Matter
Copy link
Contributor

Reproduction steps

  1. Build the chip-tool with AddressSanitizer (ASAN) enabled.

  2. Open Terminal 1 and start the chip-all-clusters-app:

    $ ./chip-all-clusters-app
  3. Open Terminal 2 and run the following commands in chip-tool:

    $ ./chip-tool interactive start
    $ pairing onnetwork-long 1 20202021 3840
    $ basicinformation read vendor-name 1 0 --allow-large-payload 1
  4. After the TCP session is established, terminate chip-all-clusters-app in Terminal 1 with Ctrl+C.

  5. Observe that chip-tool receives a FIN packet, triggering heap-use-after-free behavior.
    ASAN Log.txt

Summary

A heap-use-after-free (UAF) issue occurs when a CASESession object is deallocated but its callback remains referenced by the ActiveTCPConnectionState object. This issue introduces undefined behavior, potentially leading to security vulnerabilities under specific conditions.

Analysis and Description

The issue arises due to a mismatch in the lifecycle management of the CASESession and ActiveTCPConnectionState objects. The following sequence highlights the root cause:

  1. Creation and Callback Setup:

    During CASESession::EstablishSession, the AppTCPConnectionCallbackCtxt structure is initialized, and its connClosedCb member is set to CASESession::HandleConnectionClosed:

    CHIP_ERROR CASESession::Init(SessionManager & sessionManager, Credentials::CertificateValidityPolicy * policy,
                                 SessionEstablishmentDelegate * delegate, const ScopedNodeId & sessionEvictionHint)
    {
    	// ...
    	mTCPConnCbCtxt.connClosedCb = HandleConnectionClosed;
  2. ActiveTCPConnectionState References CASESession:

    The ActiveTCPConnectionState object is initialized during TCPBase::StartConnect, and it retains a reference to the AppTCPConnectionCallbackCtxt structure via its mAppState member:

    activeConnection->mAppState = appState;
  3. CASESession Deallocation:

    After the CASE session is successfully established, the CASESession object is deallocated. However, ActiveTCPConnectionState still holds a dangling reference to the AppTCPConnectionCallbackCtxt structure.

  4. TCP Connection Termination:

    When the TCP connection is terminated, the ActiveTCPConnectionState invokes the connClosedCb callback stored in its mAppState. Since the callback references the already-freed CASESession object, a heap-use-after-free occurs:

    if (appTCPConnCbCtxt->connClosedCb != nullptr) {
        appTCPConnCbCtxt->connClosedCb(conn, conErr);
    }

This UAF condition results from the ActiveTCPConnectionState lifecycle extending beyond the CASESession lifecycle, causing undefined behavior. If the memory previously occupied by the CASESession is overwritten with controlled data, this could lead to arbitrary code execution.

Proposed Solution

  • Refactor Callback Handling:
    • Decouple the lifecycle of CASESession from ActiveTCPConnectionState using proxy objects or weak references.

By resolving this issue, the framework can eliminate the risk of undefined behavior and proactively mitigate potential security vulnerabilities, ensuring the robustness of the system.

If further information is needed, please do not hesitate to reach out.

Bug prevalence

always

GitHub hash of the SDK that was being used

ffbc362

Platform

core

Platform Version(s)

all versions with TCP support

Anything else?

No response

@BoB13-Matter BoB13-Matter added bug Something isn't working needs triage labels Dec 5, 2024
pidarped added a commit to pidarped/connectedhomeip that referenced this issue Dec 17, 2024
Set the app_state callback object in the Connection state to null
when the CASE session object is being cleared, on top of setting the
inner callback methods to null.
This prevents the callback object from being accessed later, when the
connection is getting closed(after the CASE session has been set up and
the session object no longer exists).
pidarped added a commit to pidarped/connectedhomeip that referenced this issue Dec 18, 2024
Set the app_state callback object in the Connection state to null
when the CASE session object is being cleared, on top of setting the
inner callback methods to null.
This prevents the callback object from being accessed later, when the
connection is getting closed(after the CASE session has been set up and
the session object no longer exists).
mergify bot pushed a commit that referenced this issue Dec 18, 2024
Set the app_state callback object in the Connection state to null
when the CASE session object is being cleared, on top of setting the
inner callback methods to null.
This prevents the callback object from being accessed later, when the
connection is getting closed(after the CASE session has been set up and
the session object no longer exists).
@mergify mergify bot closed this as completed in #36879 Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants