codeql icon indicating copy to clipboard operation
codeql copied to clipboard

False positive on "Query built by concatenation with a possibly-untrusted string" - "java/concatenated-sql-query"

Open bpmarinho opened this issue 1 year ago • 1 comments
trafficstars

False positive on Query built by concatenation with a possibly-untrusted string - java/concatenated-sql-query

  • https://github.com/github/codeql/blob/ff0c1ca2d6401e63914f20b650b49c2b82cac148/java/ql/src/Security/CWE/CWE-089/SqlConcatenated.ql

We have a constant value from enum

public enum CommentType {

    REVIEW_SIMPLE_COMMENT("comment.review.simple"),
    SIMPLE_COMMENT("comment.simple");

    private final String type;

    private CommentType(String type) {
        this.type = type;
    }

    public String getType() {
        return this.type;
    }
}

Used in query

sql.append(" AND REVIEW_COMMENT.COMMENT_TYPE = '").append(CommentType.REVIEW_SIMPLE_COMMENT.getType()).append("') ");

And CodeQL is stating Query built by concatenation with a possibly-untrusted string in CommentType.REVIEW_SIMPLE_COMMENT.getType(). From my understanding the enum is immutable. Could you take a look?

bpmarinho avatar Jul 15 '24 22:07 bpmarinho

Thank you for this false positive report. Resolving this issue is not a current product priority, but we acknowledge the report and will track it internally for future consideration, or if we observe repeated instances of the same problem.

Your right that this is a false positive. The query is simply looking for SQL strings built by string concatenation without proper escaping of variables, which is usually a bad thing to do. In your case the two type values in the enum are string constants that do not contain any special SQL characters, so it is indeed safe. You can dismiss the alert, or apply some SQL string escaping to CommentType.REVIEW_SIMPLE_COMMENT.getType() before appending.

aibaars avatar Jul 17 '24 18:07 aibaars