github / github/codeql

LGTM.com - Java UnsafeTlsVersion false positive

Abierto
#4,059 3 comentarios 1 reacción 0 asignados Ver en GitHub
false-positive Java
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

**Description of the false positive**

I'm running the beta of the CodeQL GitHub actions driven code-scanning feature. Since the `gradle/gradle` repo hasn't been built recently on LGTM.com, unfortunately, I can't link directly to it at the moment.

https://github.com/gradle/gradle/blob/1e9444f521ab3bbec1a5b852c1e0c4e12fb81819/subprojects/resources-http/src/main/java/org/gradle/internal/resource/transport/http/DefaultSslContextFactory.java#L122-L122

The false positive here is from this logic:
```
SSLContext sslcontext = SSLContext.getInstance("TLS")
```

This is the query flagging this issue.

https://github.com/github/codeql/blob/66541f260bca41de238ede8d944437504268877e/java/ql/src/experimental/Security/CWE/CWE-327/UnsafeTlsVersion.ql#L1-L20

@big-guy, and I spent some time investigating this query result earlier today and _we believe_ but are not confident that this is a false positive.

Using the following sample code:

```java
public class App {
public String getGreeting() {
return "Hello world.";
}
public static void main(String[] args) throws Exception {
System.out.println(new App().getGreeting());
SSLSocket socket = connect("plugins.gradle.org", 443);
for (String protocol : socket.getEnabledProtocols()) {
System.out.println(protocol);
}
}
public static SSLSocket connect(String host, int port)
throws Exception {
SSLContext context = SSLContext.getInstance("[PICK TLS VERSION]");
context.init(null, null, null);
return (SSLSocket) context.getSocketFactory().createSocket(host, port);
}
}
```

All test are run with JDK 11.

Run with `TLS` you get the following output:
```
$ ./gradlew run
> Task :run
Hello world.
TLSv1.3
TLSv1.2
TLSv1.1
TLSv1
BUILD SUCCESSFUL in 1s
2 actionable tasks: 2 executed
```

Run with `TLSv1.3` you get the following output:
```
$ ./gradlew run
> Task :run
Hello world.
TLSv1.3
TLSv1.2
TLSv1.1
TLSv1
BUILD SUCCESSFUL in 1s
2 actionable tasks: 2 executed
```

Run with `TLSv1.2` you get the following output:
```
$ ./gradlew run
> Task :run
Hello world.
TLSv1.2
TLSv1.1
TLSv1
BUILD SUCCESSFUL in 1s
2 actionable tasks: 2 executed
```

---

Unfortunately, it seems like `SSLContext.getInstance("...")` doesn't seem to set a minimum-supported TLS version (as you might expect), but instead seems to be an upper-bound for supported version.

It seems like the vulnerability here actually exists when you pick `SSLContext.getInstance("[ANYTHING]")` but fail to restrict the TLS supported versions using `SSLSocket#setEnabledProtocols`.

Our understanding could be wrong here. I'd love if someone who has a better understanding of the Java TLS stack and TLS downgrade attacks could jump in here.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.