github / github/codeql

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

Abierto
#21,549 1 comentario 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

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

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
java
Área
security
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Tranquilo
Claridad
Bastante claro
Aptitud para principiantes
52/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.