mirror of
https://github.com/github/codeql.git
synced 2025-12-16 16:53:25 +01:00
Merge branch 'main' into redsun82/codegen-rename-dbscheme
This commit is contained in:
Paolo Tranquilli
@@ -37,7 +37,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
|
||||
{
|
||||
try
|
||||
{
|
||||
var relativePathToCsProj = Path.GetRelativePath(sourceDir, csprojFile);
|
||||
var relativePathToCsProj = Path.GetRelativePath(sourceDir, csprojFile)
|
||||
.Replace('\\', '/'); // Ensure we're generating the same hash regardless of the OS
|
||||
var name = FileUtils.ComputeHash($"{relativePathToCsProj}\n{this.GetType().Name}");
|
||||
using var tempDir = new TemporaryDirectory(Path.Join(FileUtils.GetTemporaryWorkingDirectory(out _), "source-generator"), "source generator temporary", logger);
|
||||
var analyzerConfigPath = Path.Combine(tempDir.DirInfo.FullName, $"{name}.txt");
|
||||
|
||||
@@ -81,6 +81,10 @@
|
||||
<MyOutput Value="@InputValue6" />
|
||||
</div>
|
||||
|
||||
<div>
|
||||
<MyOutput Value="@QueryParam" />
|
||||
</div>
|
||||
|
||||
@code {
|
||||
|
||||
public class Container
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
#select
|
||||
| BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | User-provided value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | User-provided value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | User-provided value |
|
||||
edges
|
||||
| BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | BlazorTest/obj/Debug/net9.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | provenance | Src:MaD:2 MaD:3 |
|
||||
| BlazorTest/obj/Debug/net9.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | provenance | Sink:MaD:1 |
|
||||
models
|
||||
| 1 | Sink: Microsoft.AspNetCore.Components; MarkupString; false; MarkupString; (System.String); ; Argument[0]; html-injection; manual |
|
||||
| 2 | Source: Microsoft.AspNetCore.Components; SupplyParameterFromQueryAttribute; false; ; ; Attribute.Getter; ReturnValue; remote; manual |
|
||||
| 3 | Summary: Microsoft.AspNetCore.Components.CompilerServices; RuntimeHelpers; false; TypeCheck<T>; (T); ; Argument[0]; ReturnValue; value; manual |
|
||||
nodes
|
||||
| BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | semmle.label | access to property Value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | semmle.label | access to property UrlParam |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | semmle.label | access to property QueryParam |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | semmle.label | access to property QueryParam : String |
|
||||
| BlazorTest/obj/Debug/net9.0/generated/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | semmle.label | call to method TypeCheck<String> : String |
|
||||
subpaths
|
||||
@@ -0,0 +1,2 @@
|
||||
query: Security Features/CWE-079/XSS.ql
|
||||
postprocess: utils/test/PrettyPrintModels.ql
|
||||
@@ -81,6 +81,10 @@
|
||||
<MyOutput Value="@InputValue6" />
|
||||
</div>
|
||||
|
||||
<div>
|
||||
<MyOutput Value="@QueryParam" />
|
||||
</div>
|
||||
|
||||
@code {
|
||||
|
||||
public class Container
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
#select
|
||||
| BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | User-provided value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | User-provided value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | User-provided value |
|
||||
edges
|
||||
| BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | test-db/working/razor/AC613014E59A413B9538FF8068364499/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | provenance | Src:MaD:2 MaD:3 |
|
||||
| test-db/working/razor/AC613014E59A413B9538FF8068364499/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | provenance | Sink:MaD:1 |
|
||||
models
|
||||
| 1 | Sink: Microsoft.AspNetCore.Components; MarkupString; false; MarkupString; (System.String); ; Argument[0]; html-injection; manual |
|
||||
| 2 | Source: Microsoft.AspNetCore.Components; SupplyParameterFromQueryAttribute; false; ; ; Attribute.Getter; ReturnValue; remote; manual |
|
||||
| 3 | Summary: Microsoft.AspNetCore.Components.CompilerServices; RuntimeHelpers; false; TypeCheck<T>; (T); ; Argument[0]; ReturnValue; value; manual |
|
||||
nodes
|
||||
| BlazorTest/Components/MyOutput.razor:5:53:5:57 | access to property Value | semmle.label | access to property Value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | semmle.label | access to property UrlParam |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | semmle.label | access to property QueryParam |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:85:23:85:32 | access to property QueryParam : String | semmle.label | access to property QueryParam : String |
|
||||
| test-db/working/razor/AC613014E59A413B9538FF8068364499/Microsoft.CodeAnalysis.Razor.Compiler/Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator/Components_Pages_TestPage_razor.g.cs:569:16:577:13 | call to method TypeCheck<String> : String | semmle.label | call to method TypeCheck<String> : String |
|
||||
subpaths
|
||||
@@ -0,0 +1,2 @@
|
||||
query: Security Features/CWE-079/XSS.ql
|
||||
postprocess: utils/test/PrettyPrintModels.ql
|
||||
@@ -81,6 +81,10 @@
|
||||
<MyOutput Value="@InputValue6" />
|
||||
</div>
|
||||
|
||||
<div>
|
||||
<MyOutput Value="@QueryParam" />
|
||||
</div>
|
||||
|
||||
@code {
|
||||
|
||||
public class Container
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
#select
|
||||
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | User-provided value |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | $@ flows to here and is written to HTML or JavaScript. | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | User-provided value |
|
||||
edges
|
||||
nodes
|
||||
| BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | semmle.label | access to property UrlParam |
|
||||
| BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | semmle.label | access to property QueryParam |
|
||||
subpaths
|
||||
@@ -0,0 +1,2 @@
|
||||
query: Security Features/CWE-079/XSS.ql
|
||||
postprocess: utils/test/PrettyPrintModels.ql
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Modeled parameter passing between Blazor parent and child components.
|
||||
@@ -147,6 +147,16 @@ predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
|
||||
pragma[inline]
|
||||
predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
|
||||
|
||||
/**
|
||||
* A module importing the modules that provide non local jump node declarations,
|
||||
* ensuring that they are visible to the taint tracking / data flow library.
|
||||
*/
|
||||
private module JumpNodes {
|
||||
private import semmle.code.csharp.frameworks.microsoft.aspnetcore.Components
|
||||
private import semmle.code.csharp.frameworks.Razor
|
||||
private import semmle.code.csharp.frameworks.NHibernate
|
||||
}
|
||||
|
||||
/**
|
||||
* A data flow node that jumps between callables. This can be extended in
|
||||
* framework code to add additional data flow steps.
|
||||
|
||||
@@ -112,6 +112,16 @@ class MicrosoftAspNetCoreComponentsComponent extends Class {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::AddComponentParameter` method.
|
||||
*/
|
||||
private class MicrosoftAspNetCoreComponentsAddComponentParameterMethod extends Method {
|
||||
MicrosoftAspNetCoreComponentsAddComponentParameterMethod() {
|
||||
this.hasFullyQualifiedName("Microsoft.AspNetCore.Components.Rendering", "RenderTreeBuilder",
|
||||
"AddComponentParameter")
|
||||
}
|
||||
}
|
||||
|
||||
private module Sources {
|
||||
private import semmle.code.csharp.security.dataflow.flowsources.Remote
|
||||
|
||||
@@ -133,3 +143,44 @@ private module Sources {
|
||||
override string getSourceType() { result = "ASP.NET Core component route parameter" }
|
||||
}
|
||||
}
|
||||
|
||||
private module JumpNodes {
|
||||
/**
|
||||
* A call to `Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder::AddComponentParameter` which
|
||||
* sets the value of a parameter.
|
||||
*/
|
||||
private class ParameterPassingCall extends Call {
|
||||
ParameterPassingCall() {
|
||||
this.getTarget() instanceof MicrosoftAspNetCoreComponentsAddComponentParameterMethod
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the property whose value is being set.
|
||||
*/
|
||||
Property getParameterProperty() {
|
||||
result.getAnAttribute() instanceof MicrosoftAspNetCoreComponentsParameterAttribute and
|
||||
exists(NameOfExpr ne | ne = this.getArgument(1) | result.getAnAccess() = ne.getAccess())
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the value being set.
|
||||
*/
|
||||
Expr getParameterValue() { result = this.getArgument(2) }
|
||||
}
|
||||
|
||||
private class ComponentParameterJump extends DataFlow::NonLocalJumpNode {
|
||||
Property prop;
|
||||
|
||||
ComponentParameterJump() {
|
||||
exists(ParameterPassingCall call |
|
||||
prop = call.getParameterProperty() and
|
||||
this.asExpr() = call.getParameterValue()
|
||||
)
|
||||
}
|
||||
|
||||
override DataFlow::Node getAJumpSuccessor(boolean preservesValue) {
|
||||
preservesValue = true and
|
||||
result.asExpr() = prop.getAnAccess()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
namespace VulnerableBlazorApp.Components
|
||||
{
|
||||
using Microsoft.AspNetCore.Components;
|
||||
|
||||
public partial class Name : Microsoft.AspNetCore.Components.ComponentBase
|
||||
{
|
||||
protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder)
|
||||
{
|
||||
if (TheName is not null)
|
||||
{
|
||||
builder.OpenElement(0, "div");
|
||||
builder.OpenElement(1, "p");
|
||||
builder.AddContent(2, (MarkupString)TheName);
|
||||
builder.CloseElement();
|
||||
builder.CloseElement();
|
||||
}
|
||||
}
|
||||
|
||||
[Parameter]
|
||||
public string TheName { get; set; }
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,50 @@
|
||||
namespace VulnerableBlazorApp.Components
|
||||
{
|
||||
using System.Collections.Generic;
|
||||
using Microsoft.AspNetCore.Components;
|
||||
|
||||
[RouteAttribute("/names/{name?}")]
|
||||
public partial class NameList : Microsoft.AspNetCore.Components.ComponentBase
|
||||
{
|
||||
protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder builder)
|
||||
{
|
||||
if (Names is not null)
|
||||
{
|
||||
builder.OpenElement(0, "div");
|
||||
builder.OpenElement(1, "ul");
|
||||
foreach (var name in Names)
|
||||
{
|
||||
builder.OpenElement(2, "li");
|
||||
builder.OpenComponent<VulnerableBlazorApp.Components.Name>(3);
|
||||
builder.AddComponentParameter(4, nameof(VulnerableBlazorApp.Components.Name.TheName), name);
|
||||
builder.CloseComponent();
|
||||
builder.CloseElement();
|
||||
}
|
||||
builder.CloseElement();
|
||||
builder.CloseElement();
|
||||
}
|
||||
|
||||
builder.OpenElement(5, "div");
|
||||
builder.OpenElement(6, "p");
|
||||
builder.AddContent(7, "Name: ");
|
||||
builder.OpenComponent<VulnerableBlazorApp.Components.Name>(8);
|
||||
builder.AddComponentParameter(9, nameof(VulnerableBlazorApp.Components.Name.TheName), Name);
|
||||
builder.CloseComponent();
|
||||
builder.CloseElement();
|
||||
}
|
||||
|
||||
[Parameter]
|
||||
public string Name { get; set; }
|
||||
|
||||
protected override void OnParametersSet()
|
||||
{
|
||||
if (Name is not null)
|
||||
{
|
||||
Names.Add(Name);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
public List<string> Names { get; set; } = new List<string>();
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,12 @@
|
||||
edges
|
||||
| NameList.cs:31:99:31:102 | access to property Name : String | Name.cs:13:53:13:59 | access to property TheName | provenance | Sink:MaD:149 |
|
||||
nodes
|
||||
| Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | semmle.label | access to property UrlParam |
|
||||
| Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | semmle.label | access to property QueryParam |
|
||||
| Name.cs:13:53:13:59 | access to property TheName | semmle.label | access to property TheName |
|
||||
| NameList.cs:31:99:31:102 | access to property Name : String | semmle.label | access to property Name : String |
|
||||
subpaths
|
||||
#select
|
||||
| Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | $@ flows to here and is written to HTML or JavaScript. | Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | User-provided value |
|
||||
| Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | $@ flows to here and is written to HTML or JavaScript. | Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | User-provided value |
|
||||
| Name.cs:13:53:13:59 | access to property TheName | NameList.cs:31:99:31:102 | access to property Name : String | Name.cs:13:53:13:59 | access to property TheName | $@ flows to here and is written to HTML or JavaScript. | NameList.cs:31:99:31:102 | access to property Name : String | User-provided value |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/CWE-079/XSS.ql
|
||||
@@ -2,3 +2,6 @@
|
||||
| Components_Pages_TestPage_razor.g.cs:138:15:138:22 | access to property UrlParam | ASP.NET Core component route parameter |
|
||||
| Components_Pages_TestPage_razor.g.cs:176:1:176:10 | access to property QueryParam | external |
|
||||
| Components_Pages_TestPage_razor.g.cs:188:18:188:27 | access to property QueryParam | external |
|
||||
| NameList.cs:31:99:31:102 | access to property Name | ASP.NET Core component route parameter |
|
||||
| NameList.cs:41:17:41:20 | access to property Name | ASP.NET Core component route parameter |
|
||||
| NameList.cs:43:27:43:30 | access to property Name | ASP.NET Core component route parameter |
|
||||
|
||||
@@ -1,15 +0,0 @@
|
||||
f = open("filename")
|
||||
... # Actions to perform on file
|
||||
f.close()
|
||||
# File only closed if actions are completed successfully
|
||||
|
||||
with open("filename") as f:
|
||||
...# Actions to perform on file
|
||||
# File always closed
|
||||
|
||||
f = open("filename")
|
||||
try:
|
||||
... # Actions to perform on file
|
||||
finally:
|
||||
f.close()
|
||||
# File always closed
|
||||
@@ -4,32 +4,30 @@
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p> If a file is opened then it should always be closed again, even if an
|
||||
exception is raised.
|
||||
Failing to ensure that all files are closed may result in failure due to too
|
||||
many open files.</p>
|
||||
|
||||
<p>When a file is opened, it should always be closed.
|
||||
</p>
|
||||
<p>A file opened for writing that is not closed when the application exits may result in data loss, where not all of the data written may be saved to the file.
|
||||
A file opened for reading or writing that is not closed may also use up file descriptors, which is a resource leak that in long running applications could lead to a failure to open additional files.
|
||||
</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure that if you open a file it is always closed on exiting the method.
|
||||
Wrap the code between the <code>open()</code> and <code>close()</code>
|
||||
functions in a <code>with</code> statement or use a <code>try...finally</code>
|
||||
statement. Using a <code>with</code> statement is preferred as it is shorter
|
||||
and more readable.</p>
|
||||
<p>Ensure that opened files are always closed, including when an exception could be raised.
|
||||
The best practice is often to use a <code>with</code> statement to automatically clean up resources.
|
||||
Otherwise, ensure that <code>.close()</code> is called in a <code>try...except</code> or <code>try...finally</code>
|
||||
block to handle any possible exceptions.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
<example>
|
||||
<p>The following code shows examples of different ways of closing a file. In the first example, the
|
||||
file is closed only if the method is exited successfully. In the other examples, the file is always
|
||||
closed on exiting the method.</p>
|
||||
<p>In the following examples, in the case marked BAD, the file may not be closed if an exception is raised. In the cases marked GOOD, the file is always closed.</p>
|
||||
|
||||
<sample src="FileNotAlwaysClosed.py" />
|
||||
<sample src="examples/FileNotAlwaysClosed.py" />
|
||||
|
||||
</example>
|
||||
<references>
|
||||
|
||||
|
||||
<li>Python Documentation: <a href="https://docs.python.org/3/tutorial/inputoutput.html#reading-and-writing-files">Reading and writing files</a>.</li>
|
||||
<li>Python Language Reference: <a href="http://docs.python.org/reference/compound_stmts.html#the-with-statement">The with statement</a>,
|
||||
<a href="http://docs.python.org/reference/compound_stmts.html#the-try-statement">The try statement</a>.</li>
|
||||
<li>Python PEP 343: <a href="http://www.python.org/dev/peps/pep-0343">The "with" Statement</a>.</li>
|
||||
|
||||
@@ -1,74 +1,26 @@
|
||||
/**
|
||||
* @name File is not always closed
|
||||
* @description Opening a file without ensuring that it is always closed may cause resource leaks.
|
||||
* @description Opening a file without ensuring that it is always closed may lead to data loss or resource leaks.
|
||||
* @kind problem
|
||||
* @tags efficiency
|
||||
* correctness
|
||||
* resources
|
||||
* quality
|
||||
* external/cwe/cwe-772
|
||||
* @problem.severity warning
|
||||
* @sub-severity high
|
||||
* @precision medium
|
||||
* @precision high
|
||||
* @id py/file-not-closed
|
||||
*/
|
||||
|
||||
import python
|
||||
import FileOpen
|
||||
import FileNotAlwaysClosedQuery
|
||||
|
||||
/**
|
||||
* Whether resource is opened and closed in in a matched pair of methods,
|
||||
* either `__enter__` and `__exit__` or `__init__` and `__del__`
|
||||
*/
|
||||
predicate opened_in_enter_closed_in_exit(ControlFlowNode open) {
|
||||
file_not_closed_at_scope_exit(open) and
|
||||
exists(FunctionValue entry, FunctionValue exit |
|
||||
open.getScope() = entry.getScope() and
|
||||
exists(ClassValue cls |
|
||||
cls.declaredAttribute("__enter__") = entry and cls.declaredAttribute("__exit__") = exit
|
||||
or
|
||||
cls.declaredAttribute("__init__") = entry and cls.declaredAttribute("__del__") = exit
|
||||
) and
|
||||
exists(AttrNode attr_open, AttrNode attrclose |
|
||||
attr_open.getScope() = entry.getScope() and
|
||||
attrclose.getScope() = exit.getScope() and
|
||||
expr_is_open(attr_open.(DefinitionNode).getValue(), open) and
|
||||
attr_open.getName() = attrclose.getName() and
|
||||
close_method_call(_, attrclose)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
predicate file_not_closed_at_scope_exit(ControlFlowNode open) {
|
||||
exists(EssaVariable v |
|
||||
BaseFlow::reaches_exit(v) and
|
||||
var_is_open(v, open) and
|
||||
not file_is_returned(v, open)
|
||||
)
|
||||
or
|
||||
call_to_open(open) and
|
||||
not exists(AssignmentDefinition def | def.getValue() = open) and
|
||||
not exists(Return r | r.getValue() = open.getNode())
|
||||
}
|
||||
|
||||
predicate file_not_closed_at_exception_exit(ControlFlowNode open, ControlFlowNode exit) {
|
||||
exists(EssaVariable v |
|
||||
exit.(RaisingNode).viableExceptionalExit(_, _) and
|
||||
not closes_arg(exit, v.getSourceVariable()) and
|
||||
not close_method_call(exit, v.getAUse().(NameNode)) and
|
||||
var_is_open(v, open) and
|
||||
v.getAUse() = exit.getAChild*()
|
||||
)
|
||||
}
|
||||
|
||||
/* Check to see if a file is opened but not closed or returned */
|
||||
from ControlFlowNode defn, string message
|
||||
from FileOpen fo, string msg
|
||||
where
|
||||
not opened_in_enter_closed_in_exit(defn) and
|
||||
(
|
||||
file_not_closed_at_scope_exit(defn) and message = "File is opened but is not closed."
|
||||
or
|
||||
not file_not_closed_at_scope_exit(defn) and
|
||||
file_not_closed_at_exception_exit(defn, _) and
|
||||
message = "File may not be closed if an exception is raised."
|
||||
)
|
||||
select defn.getNode(), message
|
||||
fileNotClosed(fo) and
|
||||
msg = "File is opened but is not closed."
|
||||
or
|
||||
fileMayNotBeClosedOnException(fo, _) and
|
||||
msg = "File may not be closed if an exception is raised."
|
||||
select fo, msg
|
||||
|
||||
150
python/ql/src/Resources/FileNotAlwaysClosedQuery.qll
Normal file
150
python/ql/src/Resources/FileNotAlwaysClosedQuery.qll
Normal file
@@ -0,0 +1,150 @@
|
||||
/** Definitions for reasoning about whether files are closed. */
|
||||
|
||||
import python
|
||||
import semmle.python.dataflow.new.internal.DataFlowDispatch
|
||||
import semmle.python.ApiGraphs
|
||||
|
||||
/** A CFG node where a file is opened. */
|
||||
abstract class FileOpenSource extends DataFlow::CfgNode { }
|
||||
|
||||
/** A call to the builtin `open` or `os.open`. */
|
||||
class FileOpenCall extends FileOpenSource {
|
||||
FileOpenCall() {
|
||||
this = [API::builtin("open").getACall(), API::moduleImport("os").getMember("open").getACall()]
|
||||
}
|
||||
}
|
||||
|
||||
private DataFlow::TypeTrackingNode fileOpenInstance(DataFlow::TypeTracker t) {
|
||||
t.start() and
|
||||
result instanceof FileOpenSource
|
||||
or
|
||||
exists(DataFlow::TypeTracker t2 | result = fileOpenInstance(t2).track(t2, t))
|
||||
}
|
||||
|
||||
/**
|
||||
* A call that returns an instance of an open file object.
|
||||
* This includes calls to methods that transitively call `open` or similar.
|
||||
*/
|
||||
class FileOpen extends DataFlow::CallCfgNode {
|
||||
FileOpen() { fileOpenInstance(DataFlow::TypeTracker::end()).flowsTo(this) }
|
||||
}
|
||||
|
||||
/** A call that may wrap a file object in a wrapper class or `os.fdopen`. */
|
||||
class FileWrapperCall extends DataFlow::CallCfgNode {
|
||||
DataFlow::Node wrapped;
|
||||
|
||||
FileWrapperCall() {
|
||||
wrapped = this.getArg(_).getALocalSource() and
|
||||
this.getFunction() = classTracker(_)
|
||||
or
|
||||
wrapped = this.getArg(0) and
|
||||
this = API::moduleImport("os").getMember("fdopen").getACall()
|
||||
or
|
||||
wrapped = this.getArg(0) and
|
||||
this = API::moduleImport("django").getMember("http").getMember("FileResponse").getACall()
|
||||
}
|
||||
|
||||
/** Gets the file that this call wraps. */
|
||||
DataFlow::Node getWrapped() { result = wrapped }
|
||||
}
|
||||
|
||||
/** A node where a file is closed. */
|
||||
abstract class FileClose extends DataFlow::CfgNode {
|
||||
/** Holds if this file close will occur if an exception is thrown at `raises`. */
|
||||
predicate guardsExceptions(DataFlow::CfgNode raises) {
|
||||
this.asCfgNode() = raises.asCfgNode().getAnExceptionalSuccessor().getASuccessor*()
|
||||
or
|
||||
// The expression is after the close call.
|
||||
// This also covers the body of a `with` statement.
|
||||
raises.asCfgNode() = this.asCfgNode().getASuccessor*()
|
||||
}
|
||||
}
|
||||
|
||||
/** A call to the `.close()` method of a file object. */
|
||||
class FileCloseCall extends FileClose {
|
||||
FileCloseCall() { exists(DataFlow::MethodCallNode mc | mc.calls(this, "close")) }
|
||||
}
|
||||
|
||||
/** A call to `os.close`. */
|
||||
class OsCloseCall extends FileClose {
|
||||
OsCloseCall() { this = API::moduleImport("os").getMember("close").getACall().getArg(0) }
|
||||
}
|
||||
|
||||
/** A `with` statement. */
|
||||
class WithStatement extends FileClose {
|
||||
WithStatement() { this.asExpr() = any(With w).getContextExpr() }
|
||||
}
|
||||
|
||||
/** Holds if an exception may be raised at `raises` if `file` is a file object. */
|
||||
private predicate mayRaiseWithFile(DataFlow::CfgNode file, DataFlow::CfgNode raises) {
|
||||
// Currently just consider any method called on `file`; e.g. `file.write()`; as potentially raising an exception
|
||||
raises.(DataFlow::MethodCallNode).getObject() = file and
|
||||
not file instanceof FileOpen and
|
||||
not file instanceof FileClose
|
||||
}
|
||||
|
||||
/** Holds if data flows from `nodeFrom` to `nodeTo` in one step that also includes file wrapper classes. */
|
||||
private predicate fileAdditionalLocalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
|
||||
exists(FileWrapperCall fw | nodeFrom = fw.getWrapped() and nodeTo = fw)
|
||||
}
|
||||
|
||||
private predicate fileLocalFlowHelper0(
|
||||
DataFlow::LocalSourceNode nodeFrom, DataFlow::LocalSourceNode nodeTo
|
||||
) {
|
||||
exists(DataFlow::Node nodeMid |
|
||||
nodeFrom.flowsTo(nodeMid) and fileAdditionalLocalFlowStep(nodeMid, nodeTo)
|
||||
)
|
||||
}
|
||||
|
||||
private predicate fileLocalFlowHelper1(
|
||||
DataFlow::LocalSourceNode nodeFrom, DataFlow::LocalSourceNode nodeTo
|
||||
) {
|
||||
fileLocalFlowHelper0*(nodeFrom, nodeTo)
|
||||
}
|
||||
|
||||
/** Holds if data flows from `source` to `sink`, including file wrapper classes. */
|
||||
pragma[inline]
|
||||
private predicate fileLocalFlow(FileOpen source, DataFlow::Node sink) {
|
||||
exists(DataFlow::LocalSourceNode mid | fileLocalFlowHelper1(source, mid) and mid.flowsTo(sink))
|
||||
}
|
||||
|
||||
/** Holds if the file opened at `fo` is closed. */
|
||||
predicate fileIsClosed(FileOpen fo) { exists(FileClose fc | fileLocalFlow(fo, fc)) }
|
||||
|
||||
/** Holds if the file opened at `fo` is returned to the caller. This makes the caller responsible for closing the file. */
|
||||
predicate fileIsReturned(FileOpen fo) {
|
||||
exists(Return ret, Expr retVal |
|
||||
(
|
||||
retVal = ret.getValue()
|
||||
or
|
||||
retVal = ret.getValue().(List).getAnElt()
|
||||
or
|
||||
retVal = ret.getValue().(Tuple).getAnElt()
|
||||
) and
|
||||
fileLocalFlow(fo, DataFlow::exprNode(retVal))
|
||||
)
|
||||
}
|
||||
|
||||
/** Holds if the file opened at `fo` is stored in a field. We assume that another method is then responsible for closing the file. */
|
||||
predicate fileIsStoredInField(FileOpen fo) {
|
||||
exists(DataFlow::AttrWrite aw | fileLocalFlow(fo, aw.getValue()))
|
||||
}
|
||||
|
||||
/** Holds if the file opened at `fo` is not closed, and is expected to be closed. */
|
||||
predicate fileNotClosed(FileOpen fo) {
|
||||
not fileIsClosed(fo) and
|
||||
not fileIsReturned(fo) and
|
||||
not fileIsStoredInField(fo)
|
||||
}
|
||||
|
||||
predicate fileMayNotBeClosedOnException(FileOpen fo, DataFlow::Node raises) {
|
||||
fileIsClosed(fo) and
|
||||
exists(DataFlow::CfgNode fileRaised |
|
||||
mayRaiseWithFile(fileRaised, raises) and
|
||||
fileLocalFlow(fo, fileRaised) and
|
||||
not exists(FileClose fc |
|
||||
fileLocalFlow(fo, fc) and
|
||||
fc.guardsExceptions(raises)
|
||||
)
|
||||
)
|
||||
}
|
||||
@@ -1,4 +1,8 @@
|
||||
/** Contains predicates concerning when and where files are opened and closed. */
|
||||
/**
|
||||
* DEPRECATED: Use FileNotAlwaysClosedQuery instead.
|
||||
* Contains predicates concerning when and where files are opened and closed.
|
||||
*/
|
||||
deprecated module;
|
||||
|
||||
import python
|
||||
import semmle.python.pointsto.Filters
|
||||
|
||||
17
python/ql/src/Resources/examples/FileNotAlwaysClosed.py
Normal file
17
python/ql/src/Resources/examples/FileNotAlwaysClosed.py
Normal file
@@ -0,0 +1,17 @@
|
||||
def bad():
|
||||
f = open("filename", "w")
|
||||
f.write("could raise exception") # BAD: This call could raise an exception, leading to the file not being closed.
|
||||
f.close()
|
||||
|
||||
|
||||
def good1():
|
||||
with open("filename", "w") as f:
|
||||
f.write("always closed") # GOOD: The `with` statement ensures the file is always closed.
|
||||
|
||||
def good2():
|
||||
f = open("filename", "w")
|
||||
try:
|
||||
f.write("always closed")
|
||||
finally:
|
||||
f.close() # GOOD: The `finally` block always ensures the file is closed.
|
||||
|
||||
@@ -1,119 +0,0 @@
|
||||
| f1_0 = open() | open | |
|
||||
| f1_1 = MethodCallsiteRefinement(f1_0) | open | |
|
||||
| f1_2 = MethodCallsiteRefinement(f1_1) | closed | exit |
|
||||
| f2_0 = open() | open | exit |
|
||||
| f3_0 = open() | open | |
|
||||
| f3_1 = MethodCallsiteRefinement(f3_0) | closed | exit |
|
||||
| f4_0 = with | closed | |
|
||||
| f4_1 = MethodCallsiteRefinement(f4_0) | closed | exit |
|
||||
| f5_0 = open() | open | |
|
||||
| f5_1 = MethodCallsiteRefinement(f5_0) | open | |
|
||||
| f5_2 = MethodCallsiteRefinement(f5_1) | closed | exit |
|
||||
| f5_3 = phi(f5_0, f5_1) | open | |
|
||||
| f6_0 = None | closed | |
|
||||
| f6_1 = open() | open | |
|
||||
| f6_2 = MethodCallsiteRefinement(f6_1) | open | |
|
||||
| f6_3 = phi(f6_0, f6_1, f6_2) | open | |
|
||||
| f6_4 = Pi(f6_2) [true] | open | |
|
||||
| f6_5 = MethodCallsiteRefinement(f6_4) | closed | |
|
||||
| f6_6 = Pi(f6_3) [true] | open | |
|
||||
| f6_7 = Pi(f6_2) [false] | closed | |
|
||||
| f6_8 = phi(f6_5, f6_7) | closed | exit |
|
||||
| f7_0 = None | closed | |
|
||||
| f7_1 = open() | open | |
|
||||
| f7_2 = MethodCallsiteRefinement(f7_1) | open | |
|
||||
| f7_3 = phi(f7_0, f7_1, f7_2) | open | |
|
||||
| f7_4 = Pi(f7_2) [true] | open | |
|
||||
| f7_5 = MethodCallsiteRefinement(f7_4) | closed | |
|
||||
| f7_6 = Pi(f7_3) [true] | open | |
|
||||
| f7_7 = Pi(f7_2) [false] | closed | |
|
||||
| f7_8 = phi(f7_5, f7_7) | closed | exit |
|
||||
| f8_0 = None | closed | |
|
||||
| f8_1 = open() | open | |
|
||||
| f8_2 = MethodCallsiteRefinement(f8_1) | open | |
|
||||
| f8_3 = phi(f8_0, f8_1, f8_2) | open | |
|
||||
| f8_4 = Pi(f8_2) [true] | closed | |
|
||||
| f8_5 = MethodCallsiteRefinement(f8_4) | closed | |
|
||||
| f8_6 = Pi(f8_3) [true] | closed | |
|
||||
| f8_7 = Pi(f8_2) [false] | open | |
|
||||
| f8_8 = phi(f8_5, f8_7) | open | exit |
|
||||
| f9_0 = None | closed | |
|
||||
| f9_1 = open() | open | |
|
||||
| f9_2 = MethodCallsiteRefinement(f9_1) | open | |
|
||||
| f9_3 = phi(f9_0, f9_1, f9_2) | open | |
|
||||
| f9_4 = Pi(f9_2) [true] | closed | |
|
||||
| f9_5 = MethodCallsiteRefinement(f9_4) | closed | |
|
||||
| f9_6 = Pi(f9_3) [true] | closed | |
|
||||
| f9_7 = Pi(f9_2) [false] | open | |
|
||||
| f9_8 = phi(f9_5, f9_7) | open | exit |
|
||||
| f10_0 = open() | open | |
|
||||
| f10_1 = MethodCallsiteRefinement(f10_0) | open | |
|
||||
| f10_2 = MethodCallsiteRefinement(f10_1) | open | |
|
||||
| f10_3 = MethodCallsiteRefinement(f10_2) | closed | |
|
||||
| f10_4 = phi(f10_0, f10_1, f10_2, f10_3) | open | |
|
||||
| f10_5 = MethodCallsiteRefinement(f10_4) | closed | |
|
||||
| f10_6 = phi(f10_3, f10_5) | closed | exit |
|
||||
| f11_0 = open() | open | |
|
||||
| f11_1 = MethodCallsiteRefinement(f11_0) | open | |
|
||||
| f11_2 = MethodCallsiteRefinement(f11_1) | open | |
|
||||
| f11_3 = MethodCallsiteRefinement(f11_2) | closed | |
|
||||
| f11_4 = phi(f11_0, f11_1, f11_2, f11_3) | open | |
|
||||
| f11_5 = MethodCallsiteRefinement(f11_4) | closed | |
|
||||
| f11_6 = phi(f11_3, f11_5) | closed | exit |
|
||||
| f12_0 = open() | open | |
|
||||
| f12_1 = MethodCallsiteRefinement(f12_0) | open | |
|
||||
| f12_2 = MethodCallsiteRefinement(f12_1) | open | |
|
||||
| f12_3 = MethodCallsiteRefinement(f12_2) | closed | |
|
||||
| f12_4 = phi(f12_0, f12_1, f12_2, f12_3) | open | |
|
||||
| f12_5 = MethodCallsiteRefinement(f12_4) | closed | |
|
||||
| f12_6 = phi(f12_3, f12_5) | closed | exit |
|
||||
| f13_0 = open() | open | |
|
||||
| f13_1 = MethodCallsiteRefinement(f13_0) | open | exit |
|
||||
| f14_0 = opener_func2() | open | |
|
||||
| f14_1 = MethodCallsiteRefinement(f14_0) | open | |
|
||||
| f14_2 = MethodCallsiteRefinement(f14_1) | closed | exit |
|
||||
| f15_0 = opener_func2() | open | |
|
||||
| f15_1 = ArgumentRefinement(f15_0) | closed | exit |
|
||||
| f16_0 = ScopeEntryDefinition | closed | |
|
||||
| f16_1 = open() | open | |
|
||||
| f16_2 = MethodCallsiteRefinement(f16_1) | open | |
|
||||
| f16_3 = MethodCallsiteRefinement(f16_2) | closed | |
|
||||
| f16_4 = phi(f16_0, f16_1, f16_2, f16_3) | open | |
|
||||
| f16_5 = phi(f16_3, f16_4) | open | exit |
|
||||
| f17_0 = open() | open | |
|
||||
| f17_1 = MethodCallsiteRefinement(f17_0) | open | |
|
||||
| f17_2 = MethodCallsiteRefinement(f17_1) | open | |
|
||||
| f17_3 = MethodCallsiteRefinement(f17_2) | closed | |
|
||||
| f17_4 = phi(f17_0, f17_1, f17_2, f17_3) | open | |
|
||||
| f17_5 = MethodCallsiteRefinement(f17_4) | closed | |
|
||||
| f17_6 = phi(f17_3, f17_5) | closed | exit |
|
||||
| f18_0 = open() | closed | |
|
||||
| f18_1 = MethodCallsiteRefinement(f18_0) | closed | exit |
|
||||
| f20_0 = open() | open | |
|
||||
| f20_1 = ArgumentRefinement(f20_0) | closed | exit |
|
||||
| f21_0 = open() | open | |
|
||||
| f21_1 = MethodCallsiteRefinement(f21_0) | open | |
|
||||
| f21_2 = MethodCallsiteRefinement(f21_1) | closed | |
|
||||
| f21_3 = phi(f21_1, f21_2) | open | |
|
||||
| f21_4 = phi(f21_0, f21_1, f21_2) | open | |
|
||||
| f21_5 = Pi(f21_3) [true] | open | |
|
||||
| f21_6 = MethodCallsiteRefinement(f21_5) | closed | |
|
||||
| f21_7 = Pi(f21_4) [true] | open | |
|
||||
| f21_8 = Pi(f21_3) [false] | closed | |
|
||||
| f21_9 = phi(f21_6, f21_8) | closed | exit |
|
||||
| f22_0 = open() | open | |
|
||||
| f22_1 = MethodCallsiteRefinement(f22_0) | open | |
|
||||
| f22_2 = MethodCallsiteRefinement(f22_1) | closed | |
|
||||
| f22_3 = phi(f22_1, f22_2) | open | |
|
||||
| f22_4 = phi(f22_0, f22_1, f22_2) | open | |
|
||||
| f22_5 = Pi(f22_3) [true] | closed | |
|
||||
| f22_6 = MethodCallsiteRefinement(f22_5) | closed | |
|
||||
| f22_7 = Pi(f22_4) [true] | closed | |
|
||||
| f22_8 = Pi(f22_3) [false] | open | |
|
||||
| f22_9 = phi(f22_6, f22_8) | open | exit |
|
||||
| f_0 = FunctionExpr | closed | exit |
|
||||
| file_0 = open() | open | |
|
||||
| file_1 = open() | open | |
|
||||
| file_2 = None | closed | |
|
||||
| file_3 = phi(file_0, file_1, file_2) | open | exit |
|
||||
| fp_0 = ParameterDefinition | closed | exit |
|
||||
@@ -1,14 +0,0 @@
|
||||
import python
|
||||
import Resources.FileOpen
|
||||
|
||||
from EssaVariable v, EssaDefinition def, string open, string exit
|
||||
where
|
||||
def = v.getDefinition() and
|
||||
v.getSourceVariable().getName().charAt(0) = "f" and
|
||||
(
|
||||
var_is_open(v, _) and open = "open"
|
||||
or
|
||||
not var_is_open(v, _) and open = "closed"
|
||||
) and
|
||||
if BaseFlow::reaches_exit(v) then exit = "exit" else exit = ""
|
||||
select v.getRepresentation() + " = " + v.getDefinition().getRepresentation(), open, exit
|
||||
@@ -1,9 +0,0 @@
|
||||
| resources_test.py:4:10:4:25 | open() | File may not be closed if an exception is raised. |
|
||||
| resources_test.py:9:10:9:25 | open() | File is opened but is not closed. |
|
||||
| resources_test.py:49:14:49:29 | open() | File is opened but is not closed. |
|
||||
| resources_test.py:58:14:58:29 | open() | File is opened but is not closed. |
|
||||
| resources_test.py:79:11:79:26 | open() | File may not be closed if an exception is raised. |
|
||||
| resources_test.py:108:11:108:20 | open() | File is opened but is not closed. |
|
||||
| resources_test.py:112:11:112:28 | opener_func2() | File may not be closed if an exception is raised. |
|
||||
| resources_test.py:129:15:129:24 | open() | File is opened but is not closed. |
|
||||
| resources_test.py:237:11:237:26 | open() | File is opened but is not closed. |
|
||||
@@ -1 +0,0 @@
|
||||
Resources/FileNotAlwaysClosed.ql
|
||||
@@ -1,12 +1,12 @@
|
||||
#File not always closed
|
||||
|
||||
def not_close1():
|
||||
f1 = open("filename")
|
||||
f1 = open("filename") # $ notClosedOnException
|
||||
f1.write("Error could occur")
|
||||
f1.close()
|
||||
f1.close()
|
||||
|
||||
def not_close2():
|
||||
f2 = open("filename")
|
||||
f2 = open("filename") # $ notClosed
|
||||
|
||||
def closed3():
|
||||
f3 = open("filename")
|
||||
@@ -46,19 +46,19 @@ def closed7():
|
||||
def not_closed8():
|
||||
f8 = None
|
||||
try:
|
||||
f8 = open("filename")
|
||||
f8 = open("filename") # $ MISSING:notClosedOnException
|
||||
f8.write("Error could occur")
|
||||
finally:
|
||||
if f8 is None:
|
||||
if f8 is None: # We don't precisely consider this condition, so this result is MISSING. However, this seems uncommon.
|
||||
f8.close()
|
||||
|
||||
def not_closed9():
|
||||
f9 = None
|
||||
try:
|
||||
f9 = open("filename")
|
||||
f9 = open("filename") # $ MISSING:notAlwaysClosed
|
||||
f9.write("Error could occur")
|
||||
finally:
|
||||
if not f9:
|
||||
if not f9: # We don't precisely consider this condition, so this result is MISSING.However, this seems uncommon.
|
||||
f9.close()
|
||||
|
||||
def not_closed_but_cant_tell_locally():
|
||||
@@ -76,19 +76,19 @@ def closed10():
|
||||
|
||||
#Not closed by handling the wrong exception
|
||||
def not_closed11():
|
||||
f11 = open("filename")
|
||||
f11 = open("filename") # $ MISSING:notAlwaysClosed
|
||||
try:
|
||||
f11.write("IOError could occur")
|
||||
f11.write("IOError could occur")
|
||||
f11.close()
|
||||
except AttributeError:
|
||||
except AttributeError: # We don't consider the type of exception handled here, so this result is MISSING.
|
||||
f11.close()
|
||||
|
||||
def doesnt_raise():
|
||||
def doesnt_raise(*args):
|
||||
pass
|
||||
|
||||
def mostly_closed12():
|
||||
f12 = open("filename")
|
||||
f12 = open("filename")
|
||||
try:
|
||||
f12.write("IOError could occur")
|
||||
f12.write("IOError could occur")
|
||||
@@ -105,11 +105,11 @@ def opener_func2(name):
|
||||
return t1
|
||||
|
||||
def not_closed13(name):
|
||||
f13 = open(name)
|
||||
f13 = open(name) # $ notClosed
|
||||
f13.write("Hello")
|
||||
|
||||
def may_not_be_closed14(name):
|
||||
f14 = opener_func2(name)
|
||||
f14 = opener_func2(name) # $ notClosedOnException
|
||||
f14.write("Hello")
|
||||
f14.close()
|
||||
|
||||
@@ -120,13 +120,13 @@ def closer2(t3):
|
||||
closer1(t3)
|
||||
|
||||
def closed15():
|
||||
f15 = opener_func2()
|
||||
closer2(f15)
|
||||
f15 = opener_func2() # $ SPURIOUS:notClosed
|
||||
closer2(f15) # We don't detect that this call closes the file, so this result is SPURIOUS.
|
||||
|
||||
|
||||
def may_not_be_closed16(name):
|
||||
try:
|
||||
f16 = open(name)
|
||||
f16 = open(name) # $ notClosedOnException
|
||||
f16.write("Hello")
|
||||
f16.close()
|
||||
except IOError:
|
||||
@@ -138,13 +138,13 @@ def may_raise():
|
||||
|
||||
#Not handling all exceptions, but we'll tolerate the false negative
|
||||
def not_closed17():
|
||||
f17 = open("filename")
|
||||
f17 = open("filename") # $ MISSING:notClosedOnException
|
||||
try:
|
||||
f17.write("IOError could occur")
|
||||
f17.write("IOError could occur")
|
||||
may_raise("ValueError could occur") # FN here.
|
||||
f17.close()
|
||||
except IOError:
|
||||
except IOError: # We don't detect that a ValueErrror could be raised that isn't handled here, so this result is MISSING.
|
||||
f17.close()
|
||||
|
||||
#ODASA-3779
|
||||
@@ -234,13 +234,47 @@ def closed21(path):
|
||||
|
||||
|
||||
def not_closed22(path):
|
||||
f22 = open(path, "wb")
|
||||
f22 = open(path, "wb") # $ MISSING:notClosedOnException
|
||||
try:
|
||||
f22.write(b"foo")
|
||||
may_raise()
|
||||
if foo:
|
||||
f22.close()
|
||||
finally:
|
||||
if f22.closed: # Wrong sense
|
||||
if f22.closed: # We don't precisely consider this condition, so this result is MISSING. However, this seems uncommon.
|
||||
f22.close()
|
||||
|
||||
def not_closed23(path):
|
||||
f23 = open(path, "w") # $ notClosed
|
||||
wr = FileWrapper(f23)
|
||||
|
||||
def closed24(path):
|
||||
f24 = open(path, "w")
|
||||
try:
|
||||
f24.write("hi")
|
||||
except:
|
||||
pass
|
||||
f24.close()
|
||||
|
||||
def closed25(path):
|
||||
from django.http import FileResponse
|
||||
return FileResponse(open(path))
|
||||
|
||||
import os
|
||||
def closed26(path):
|
||||
fd = os.open(path)
|
||||
os.close(fd)
|
||||
|
||||
def not_closed27(path):
|
||||
fd = os.open(path, "w") # $notClosedOnException
|
||||
f27 = os.fdopen(fd, "w")
|
||||
f27.write("hi")
|
||||
f27.close()
|
||||
|
||||
def closed28(path):
|
||||
fd = os.open(path, os.O_WRONLY)
|
||||
f28 = os.fdopen(fd, "w")
|
||||
try:
|
||||
f28.write("hi")
|
||||
finally:
|
||||
f28.close()
|
||||
@@ -0,0 +1,25 @@
|
||||
import python
|
||||
import Resources.FileNotAlwaysClosedQuery
|
||||
import utils.test.InlineExpectationsTest
|
||||
|
||||
module MethodArgTest implements TestSig {
|
||||
string getARelevantTag() { result = ["notClosed", "notClosedOnException"] }
|
||||
|
||||
predicate hasActualResult(Location location, string element, string tag, string value) {
|
||||
exists(DataFlow::CfgNode el, FileOpen fo |
|
||||
el = fo and
|
||||
element = el.toString() and
|
||||
location = el.getLocation() and
|
||||
value = "" and
|
||||
(
|
||||
fileNotClosed(fo) and
|
||||
tag = "notClosed"
|
||||
or
|
||||
fileMayNotBeClosedOnException(fo, _) and
|
||||
tag = "notClosedOnException"
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
import MakeTest<MethodArgTest>
|
||||
Reference in New Issue
Block a user