github / github/codeql

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

Đang mở
#21,549 1 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
CodeQL
Star
10.1k
Fork
2.1k
Merge trung bình
2 ngày 15 giờ
Pull request đã merge (30 ngày)
141

Mô tả

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

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
java
Lĩnh vực
security
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
52/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.