-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
False Negative: ImproperWebViewCertificateValidation.ql misses unconditional handler.proceed() calls once they are hidden behind aliases or tiny wrappers.
Version
codeql 2.24.3
Checker
- Checker id:
Security/CWE/CWE-295/ImproperWebViewCertificateValidation.ql - Checker description: This checker detects Android WebViewClient.onReceivedSslError method implementations that unconditionally accept all SSL certificates by calling SslErrorHandler.proceed without proper validation.
Description of the false negative
These samples still accept every certificate error in onReceivedSslError(...). The code only changes how proceed() is reached: through an alias, a helper method, or a trivially true guard.
Affected test cases
PosCase1_Var1.java
Aliasing the handler does not change the fact that the code still calls proceed() unconditionally.
// A class extends android.webkit.WebViewClient and overrides onReceivedSslError, directly calling handler.proceed() without any conditional checks should be flagged as unconditional SSL error acceptance.
package scensct.var.pos;
import android.webkit.WebViewClient;
import android.webkit.SslErrorHandler;
import android.net.http.SslError;
public class PosCase1_Var1 extends WebViewClient {
@Override
public void onReceivedSslError(android.webkit.WebView webView, SslErrorHandler sslHandler, SslError sslError) {
// Renamed parameters and added a temporary variable
SslErrorHandler h = sslHandler;
h.proceed();
}
}PosCase1_Var3.java
Moving the proceed() call into a helper does not make the certificate validation any less unsafe.
// A class extends android.webkit.WebViewClient and overrides onReceivedSslError, directly calling handler.proceed() without any conditional checks should be flagged as unconditional SSL error acceptance.
package scensct.var.pos;
import android.webkit.WebViewClient;
import android.webkit.SslErrorHandler;
import android.net.http.SslError;
public class PosCase1_Var3 extends WebViewClient {
@Override
public void onReceivedSslError(android.webkit.WebView view, SslErrorHandler handler, SslError error) {
// Extracted the call to a private helper method that still unconditionally proceeds
handleSslError(handler);
}
private void handleSslError(SslErrorHandler handler) {
handler.proceed();
}
}PosCase1_Var4.java
This still accepts every certificate without validation; the wrapper method only hides the call shape.
// A class extends android.webkit.WebViewClient and overrides onReceivedSslError, directly calling handler.proceed() without any conditional checks should be flagged as unconditional SSL error acceptance.
package scensct.var.pos;
import android.webkit.WebViewClient;
import android.webkit.SslErrorHandler;
import android.net.http.SslError;
public class PosCase1_Var4 extends WebViewClient {
@Override
public void onReceivedSslError(android.webkit.WebView view, SslErrorHandler handler, SslError error) {
// Added dead code that does not affect the unconditional call
int unused = 42;
if (unused > 0) {
// This always executes
handler.proceed();
}
// No else branch, so call is still unconditional
}
}PosCase1_Var5.java
The same handler still reaches proceed(). The extra local variable should not hide the issue.
// A class extends android.webkit.WebViewClient and overrides onReceivedSslError, directly calling handler.proceed() without any conditional checks should be flagged as unconditional SSL error acceptance.
package scensct.var.pos;
import android.webkit.WebViewClient;
import android.webkit.SslErrorHandler;
import android.net.http.SslError;
public class PosCase1_Var5 extends WebViewClient {
@Override
public void onReceivedSslError(android.webkit.WebView view, SslErrorHandler handler, SslError error) {
// Split the call across multiple statements with a temporary
SslErrorHandler temp = handler;
temp.proceed();
// Added a redundant statement that does not change behavior
temp.toString();
}
}Cause analysis
The query appears to require a very direct handler.proceed() call in the override body. As soon as the same unconditional acceptance is expressed through a local alias, a private helper, or a trivial always-true branch, the match falls apart.
That is a real blind spot. Unsafe WebView certificate handling is often hidden behind exactly these tiny abstractions.
References
None known.