dotnet / dotnet/aspnetcore

Unreachable null check in HttpsConnectionMiddleware certificate validation path

Open
#67,212 0 comments 0 reactions 0 assignees View on GitHub
area-middleware
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.