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 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
CodeQL
星标
10.1k
派生
2.1k
平均合并
2 天 15 小时
30 天内合并 PR
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 摘要。