Skip to content

False Negative: ImproperWebViewCertificateValidation.ql misses unconditional handler.proceed() calls once they are hidden behind aliases or tiny wrappers. #21549

@Carlson-JLQ

Description

@Carlson-JLQ

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions