Unreachable null check in HttpsConnectionMiddleware certificate validation path
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 290
Description
Description
While analyzing ASP.NET Core sources with the SVACE static analyzer, I found a possible unreachable code fragment in `HttpsConnectionMiddleware`.
File:
[`src/Servers/Kestrel/Core/src/Middleware/HttpsConnectionMiddleware.cs`](https://svacer.community.ispras.ru/mode/review/project/2f3915a4-3073-4ded-9b81-4c38004a5c9a/branch/6ddcf5a0-1a90-4c50-80dd-82c2d27678a7/snapshot/78a3c82a-d01f-4967-bedf-6171da636b0d/marker/eyJtYXJrZXJJRCI6ICI2ODI3NTJhOC03NjY2LTQxNTUtYWZlZi0wZjNiYjE1ZGI4OTAiLCAiZmlsZSI6ICIvc3JjL1NlcnZlcnMvS2VzdHJlbC9Db3JlL3NyYy9NaWRkbGV3YXJlL0h0dHBzQ29ubmVjdGlvbk1pZGRsZXdhcmUuY3MifQ==)
[src/Shared/RangeHelper/RangeHelper.cs](https://svacer.community.ispras.ru/r/d8oebjdeeg6c2komh2t0)
ASP.NET Core 6.0.36
The following logic first checks whether `certificate` is null:
```csharp
if (certificate == null)
{
return clientCertificateMode != ClientCertificateMode.RequireCertificate;
}
```
and later performs:
```csharp
var certificate2 = ConvertToX509Certificate2(certificate);
if (certificate2 == null)
{
return false;
}
```
According to the implementation of `ConvertToX509Certificate2`, it returns `null` only when the input certificate is `null`.
Since the `certificate == null` case has already been handled earlier, the subsequent check
```csharp
if (certificate2 == null)
{
return false;
}
```
appears to be unreachable.
As a result, this looks like redundant defensive code that can never be executed.
A possible simplification would be removing the unnecessary null check after the conversion.
This issue does not appear to have security impact, but it may be worth cleaning up the code path and eliminating unreachable logic.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Contributor guide
Assessment
This issue has not been assessed yet.