CodeQL queries 1.24

Skip to end of metadata
Go to start of metadata

Name: File is not always closed

Description: Opening a file without ensuring that it is always closed may cause resource leaks.

ID: py/file-not-closed

Kind: problem

Severity: warning

Precision: medium

Query: FileNotAlwaysClosed.ql
/**
 * @name File is not always closed
 * @description Opening a file without ensuring that it is always closed may cause resource leaks.
 * @kind problem
 * @tags efficiency
 *       correctness
 *       resources
 *       external/cwe/cwe-772
 * @problem.severity warning
 * @sub-severity high
 * @precision medium
 * @id py/file-not-closed
 */

import python
import FileOpen

/** 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(FunctionObject entry, FunctionObject exit |
        open.getScope() = entry.getFunction() and
        exists(ClassObject 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.getFunction() and
            attrclose.getScope() = exit.getFunction() 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
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

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.

Recommendation

Ensure that if you open a file it is always closed on exiting the method. Wrap the code between the open() and close() functions in a with statement or use a try...finally statement. Using a with statement is preferred as it is shorter and more readable.

Example

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.

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

References