Skip to content

Commit

Permalink
Merge pull request #18865 from carldybdahl-microsoft/csharp/path-combine
Browse files Browse the repository at this point in the history
Add CodeQL recommendation against Path.Combine
  • Loading branch information
michaelnebel authored Mar 4, 2025
2 parents 7f56c67 + 2f7cdf1 commit 96c0ca8
Showing 7 changed files with 56 additions and 0 deletions.
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
2 changes: 2 additions & 0 deletions csharp/ql/test/query-tests/Bad Practices/Path Combine/options
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

0 comments on commit 96c0ca8

Please sign in to comment.