RocketChat / RocketChat/Rocket.Chat.ReactNative
Android: pinned OkHttpClient lazy fallback can permanently bind to non-pinned client
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 2.4k
- Forks
- 1.5k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 90
Description
Problem
MediaCallsAnswerRequest.kt:45-53 and DDPClient.kt:29 both initialize the OkHttp client via a companion by lazy block:
```kotlin
private val httpClient: OkHttpClient by lazy {
SSLPinningTurboModule.getSharedOkHttpClient() ?: OkHttpClient()
}
```
If a VoIP notification (or DDP message) arrives during cold start before the user has selected a certificate, the lazy initializer triggers when the shared pinned client is still null, permanently binding to a plain OkHttpClient() for the process lifetime. Subsequent calls — including manual certificate selection later in the session — reuse the cached non-pinned fallback and silently bypass SSL pinning.
Where this surfaces
- VoIP accept REST (
MediaCallsAnswerRequest.fetch) when invoked from a PushKit/FCM cold-start handler - DDPClient connections under similar cold-start conditions
Suggested fix
Resolve the shared client at each call site rather than at lazy initialization, OR fail-fast with onResult(false) when getSharedOkHttpClient() returns null.
Background
Spotted by CodeRabbit on https://github.com/RocketChat/Rocket.Chat.ReactNative/pull/7245 — this is a pre-existing bug, not introduced by that PR. Filing separately so it doesn't block the release-readiness queue.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with MediaCallsAnswerRequest.kt:45-53 and DDPClient.kt:29, then trace how their companion lazy clients are used by MediaCallsAnswerRequest.fetch and DDPClient connections. Reproduce the cold-start path with no selected certificate, then select one and verify subsequent calls no longer reuse a plain client or silently bypass SSL pinning.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, kotlin
- Domain
- mobile, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100