Semmle 1.20
Skip to end of metadata
Go to start of metadata

Name: Open descriptor may not be closed

Description: Failing to close resources in the function that opened them makes it difficult to avoid and detect resource leaks.

ID: cpp/descriptor-may-not-be-closed

Kind: problem

Severity: warning

Query: DescriptorMayNotBeClosed.ql
/**
 * @name Open descriptor may not be closed
 * @description Failing to close resources in the function that opened them makes it difficult to avoid and detect resource leaks.
 * @kind problem
 * @id cpp/descriptor-may-not-be-closed
 * @problem.severity warning
 * @tags efficiency
 *       security
 *       external/cwe/cwe-775
 */
import semmle.code.cpp.pointsto.PointsTo
import Negativity

predicate closeCall(FunctionCall fc, Variable v)
{
  (fc.getTarget().hasQualifiedName("close") and v.getAnAccess() = fc.getArgument(0))
  or
  exists(FunctionCall midcall, Function mid, int arg |
    fc.getArgument(arg) = v.getAnAccess() and
    fc.getTarget() = mid and
    midcall.getEnclosingFunction() = mid and
    closeCall(midcall, mid.getParameter(arg)))
}

predicate openDefinition(LocalScopeVariable v, ControlFlowNode def)
{
  exists(Expr expr |
      exprDefinition(v, def, expr) and allocateDescriptorCall(expr))
}

predicate openReaches(ControlFlowNode def, ControlFlowNode node)
{
  exists(LocalScopeVariable v |
    openDefinition(v, def) and node = def.getASuccessor())
  or
  exists(LocalScopeVariable v, ControlFlowNode mid |
    openDefinition(v, def) and
    openReaches(def, mid) and
    not(errorSuccessor(v, mid)) and
    not(closeCall(mid, v)) and
    not(assignedToFieldOrGlobal(v, mid)) and
    node = mid.getASuccessor())
}

predicate assignedToFieldOrGlobal(LocalScopeVariable v, Assignment assign)
{
  exists(Variable external |
    assign.getRValue() = v.getAnAccess() and
    assign.getLValue().(VariableAccess).getTarget() = external and
    (external instanceof Field or external instanceof GlobalVariable))
}

from LocalScopeVariable v, ControlFlowNode def, ReturnStmt ret
where openDefinition(v, def)
  and openReaches(def, ret)
  and checkedSuccess(v, ret)
  and not(ret.getExpr().getAChild*() = v.getAnAccess())
  and exists(ReturnStmt other | other.getExpr() = v.getAnAccess())
select ret,
  "Descriptor assigned to '" + v.getName().toString() + "' (line " +
  def.getLocation().getStartLine().toString() + ") may not be closed."

This query looks at functions that return file or socket descriptors, but may return an error value before actually closing the resource. This can occur when an operation performed on the open descriptor fails, and the function returns with an error before it closes the open resource. An improperly handled error could cause the function to leak resource descriptors. Failing to close resources in the function that opened them also makes it more difficult to detect leaks.

This check is an approximation, so some results may not be actual defects in the program. It is not possible in general to compute the actual branch taken in conditional statements such as "if" without running the program with all possible input data. This means that it is not possible to determine if a particular statement is going to be executed.

Recommendation

When an error occurs, ensure that the function frees all the resources it holds open.

Example

In the example below, the sockfd socket may remain open if an error is triggered. The code should be updated to ensure that the socket is always closed when when the function ends.

int f() {
	try {
		int sockfd = socket(AF_INET, SOCK_STREAM, 0);
		do_stuff(sockfd);
		return sockfd; //if there are no exceptions, the socket is returned
	} catch (int do_stuff_exception) {
		return -1; //return error value, but sockfd may still be open
	}
}

References