Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add CodeQL recommendation against Path.Combine #18865

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions csharp/ql/src/Bad Practices/PathCombine.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p><code>Path.Combine</code> may silently drop its earlier arguments if its later arguments are absolute paths. E.g. <code>Path.Combine("C:\\Users\\Me\\Documents", "C:\\Program Files\\") == "C:\\Program Files"</code>.</p>

</overview>
<recommendation>
<p>Use <code>Path.Join</code> instead.</p>
</recommendation>
<references>

<li>Microsoft Learn, .NET API browser, <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.path.combine?view=net-9.0">Path.Combine</a>.</li>
<li>Microsoft Learn, .NET API browser, <a href="https://learn.microsoft.com/en-us/dotnet/api/system.io.path.join?view=net-9.0">Path.Join</a>.</li>

</references>
</qhelp>
16 changes: 16 additions & 0 deletions csharp/ql/src/Bad Practices/PathCombine.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* @name Call to System.IO.Path.Combine
* @description Finds calls to System.IO.Path's Combine method
* @kind problem
* @problem.severity recommendation
* @precision very-high
* @id cs/path-combine
* @tags reliability
*/

import csharp
import semmle.code.csharp.frameworks.System

from MethodCall call
where call.getTarget().hasFullyQualifiedName("System.IO", "Path", "Combine")
select call, "Call to 'System.IO.Path.Combine'."
4 changes: 4 additions & 0 deletions csharp/ql/src/change-notes/2025-02-26-path-combine.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `csharp/path-combine`, to recommend against the `Path.Combine` method due to it silently discarding its earlier parameters if later parameters are rooted.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using System.IO;

class PathCombine
{
void bad()
{
Path.Combine(@"C:\Users", @"C:\Program Files");
}

void good()
{
Path.Join(@"C:\Users", @"C:\Program Files");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| PathCombine.cs:7:9:7:54 | call to method Combine | Call to 'System.IO.Path.Combine'. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Bad Practices/PathCombine.ql

Check warning

Code scanning / CodeQL

Query test without inline test expectations Warning test

Query test does not use inline test expectations.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
semmle-extractor-options: /nostdlib /noconfig
semmle-extractor-options: --load-sources-from-project:${testdir}/../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj