Skip to content

Commit 5b0fd45

Browse files
committed
Fix parallel TLS hook issue
It's unclear why, but in some scenarios the hook would reliably hit a null pointer within the native realCallback() call if the call is made while another is already in progress (even though without Frida that's presumably happening just fine, and they're different cb pointers etc etc). We could use Frida's exclusive scheduling to fix this, but that raises the risk of a deadlock here a bit in, so instead we do a very simple locking setup with a polling unlock. Very quick & rough but works nicely, and allows reentrant locks in a single thread in case some apps use SSL to verify SSL somehow. Hard to imagine a cross-thread deadlock here so hopefully that'll be sufficient...
1 parent e99740b commit 5b0fd45

File tree

1 file changed

+20
-6
lines changed

1 file changed

+20
-6
lines changed

native-tls-hook.js

+20-6
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,27 @@ function patchTargetLib(targetLib) {
103103
? new NativeFunction(realCallbackAddr, 'int', ['pointer','pointer'])
104104
: () => SSL_VERIFY_INVALID; // Callback can be null - treat as invalid (=our validation only)
105105

106+
let pendingCheckThreads = new Set();
107+
106108
const hookedCallback = new NativeCallback(function (ssl, out_alert) {
107-
let realResult = false;
109+
let realResult = false; // False = not yet called, 0/1 = call result
110+
111+
const threadId = Process.getCurrentThreadId();
112+
const alreadyHaveLock = pendingCheckThreads.has(threadId);
113+
114+
// We try to have only one thread running these checks at a time, as parallel calls
115+
// here on the same underlying callback seem to crash in some specific scenarios
116+
while (pendingCheckThreads.size > 0 && !alreadyHaveLock) {
117+
Thread.sleep(0.01);
118+
}
119+
pendingCheckThreads.add(threadId);
108120

109121
if (targetLib !== 'libboringssl.dylib') {
110122
// Cronet assumes its callback is always called, and crashes if not. iOS's BoringSSL
111123
// meanwhile seems to use some negative checks in its callback, and rejects the
112124
// connection independently of the return value here if it's called with a bad cert.
113125
// End result: we *only sometimes* proactively call the callback.
114-
realResult = realCallback(ssl, out_alert)
126+
realResult = realCallback(ssl, out_alert);
115127
}
116128

117129
// Extremely dumb certificate validation: we accept any chain where the *exact* CA cert
@@ -138,16 +150,18 @@ function patchTargetLib(targetLib) {
138150
const certData = new Uint8Array(certPointer.readByteArray(certDataLength));
139151

140152
if (certData.every((byte, j) => CERT_DER[j] === byte)) {
153+
if (!alreadyHaveLock) pendingCheckThreads.delete(threadId);
141154
return SSL_VERIFY_OK;
142155
}
143156
}
144157

145158
// No matched peer - fallback to the provided callback instead:
146-
if (realResult !== false) {
147-
return realResult;
148-
} else {
149-
return realCallback(ssl, out_alert);
159+
if (realResult === false) { // Haven't called it yet
160+
realResult = realCallback(ssl, out_alert);
150161
}
162+
163+
if (!alreadyHaveLock) pendingCheckThreads.delete(threadId);
164+
return realResult;
151165
}, 'int', ['pointer','pointer']);
152166

153167
verificationCallbackCache[realCallbackAddr] = hookedCallback;

0 commit comments

Comments
 (0)