github / github/codeql

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

オープン
#21,549 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
CodeQL
スター
10.1k
フォーク
2.1k
平均マージ
2日 15時間
マージ済み PR(30日)
141

説明

# 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.

```java
// 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.

```java
// 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.

```java
// 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.

```java
// 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.

コントリビューションガイド

コントリビューションガイドを開く

調査の方向性

Start with Security/CWE/CWE-295/ImproperWebViewCertificateValidation.ql and review how the checker identifies onReceivedSslError implementations and direct proceed() calls. Reproduce the behavior with PosCase1_Var1.java, PosCase1_Var3.java, PosCase1_Var4.java, and PosCase1_Var5.java; done means these unsafe variants are detected without regressing existing cases.

索引モデルが issue の本文から書いたものです。

評価

技術スタック
java
領域
security
issue の種類
バグ
難易度
4/5
見積もり時間
3〜5日
活発さ
静か
明瞭さ
おおむね明確
初心者へのやさしさ
52/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。