False Negative: ImproperWebViewCertificateValidation.ql misses unconditional `handler.proceed()` calls once they are hidden behind aliases or tiny wrappers.
- 主要言語
- 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