github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.66k stars 1.53k forks source link

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

Open bpmarinho opened 3 months ago

bpmarinho commented 3 months ago

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

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?

aibaars commented 3 months ago

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.