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

About c++ CWE-022 example files, CWE-022 ql file does not seem to detect vulnerabilities. #14359

Open ljs9904ljs opened 1 year ago

ljs9904ljs commented 1 year ago

Description of the issue

<< codeql version >>

CodeQL command-line toolchain release 2.14.6.

<< codeql command test context >>

(a-1) Cloning codeql 'main' branch git clone https://github.com/github/codeql.git codeql-main-clone

(a-2) Use reference c file in the CWE-022 directory /home/junseok/workdir/codeql is my directory path.

/home/junseok/workdir/codeql/codeql-main-clone/cpp/ql/src/Security/CWE/CWE-022

CWE-022 directory contains... TaintedPath.c TaintedPath.qhelp TaintedPath.ql

(a-3) Add makefile to the CWE-022 directory for database creation image

<< makefile >>

CC = gcc
CFLAGS = -Wall

SRCS=$(wildcard *.c)

OBJS=$(SRCS:.c=.o)

all: $(OBJS)

${OBJS} : %.o: %.c Makefile
    -$(CC) $(CFLAGS) -c $<

(b-1) database creation

codeql database create \
--language=cpp               \
--command=make               \
--source-root=/home/junseok/workdir/codeql/codeql-main-clone/cpp/ql/src/Security/CWE/CWE-022         \
--overwrite  \
/home/junseok/workdir/codeql-database               \
--verbosity=errors 2>&1 > build.log 

(b-2) Print build.log (database creation log file)

[2023-10-03 16:08:58] [build-stdout] gcc -Wall -c TaintedPath.c
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c: In function ‘main’:
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:5:21: error: ‘FILENAME_MAX’ undeclared (first use in this function)
[2023-10-03 16:08:58] [build-stderr]      char fileBuffer[FILENAME_MAX] = "/home/";
[2023-10-03 16:08:58] [build-stderr]                      ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:5:21: note: each undeclared identifier is reported only once for each function it appears in
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:7:5: error: unknown type name ‘size_t’
[2023-10-03 16:08:58] [build-stderr]      size_t len = strlen(fileName);
[2023-10-03 16:08:58] [build-stderr]      ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:7:5: warning: implicit declaration of function ‘strlen’ [-Wimplicit-function-declaration]
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:7:18: warning: incompatible implicit declaration of built-in function ‘strlen’ [enabled by default]
[2023-10-03 16:08:58] [build-stderr]      size_t len = strlen(fileName);
[2023-10-03 16:08:58] [build-stderr]                   ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:8:5: warning: implicit declaration of function ‘strncat’ [-Wimplicit-function-declaration]
[2023-10-03 16:08:58] [build-stderr]      strncat(fileName+len, userAndFile, FILENAME_MAX-len-1);
[2023-10-03 16:08:58] [build-stderr]      ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:8:5: warning: incompatible implicit declaration of built-in function ‘strncat’ [enabled by default]
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:10:5: warning: implicit declaration of function ‘fopen’ [-Wimplicit-function-declaration]
[2023-10-03 16:08:58] [build-stderr]      fopen(fileName, "wb+");
[2023-10-03 16:08:58] [build-stderr]      ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:5:10: warning: unused variable ‘fileBuffer’ [-Wunused-variable]
[2023-10-03 16:08:58] [build-stderr]      char fileBuffer[FILENAME_MAX] = "/home/";
[2023-10-03 16:08:58] [build-stderr]           ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:16:5: error: unknown type name ‘size_t’
[2023-10-03 16:08:58] [build-stderr]      size_t len = strlen(fileName);
[2023-10-03 16:08:58] [build-stderr]      ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:16:18: warning: incompatible implicit declaration of built-in function ‘strlen’ [enabled by default]
[2023-10-03 16:08:58] [build-stderr]      size_t len = strlen(fileName);
[2023-10-03 16:08:58] [build-stderr]                   ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:19:5: warning: incompatible implicit declaration of built-in function ‘strncat’ [enabled by default]
[2023-10-03 16:08:58] [build-stderr]      strncat(fileName+len, fixed, FILENAME_MAX-len-1);
[2023-10-03 16:08:58] [build-stderr]      ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:14:10: warning: unused variable ‘fileBuffer’ [-Wunused-variable]
[2023-10-03 16:08:58] [build-stderr]      char fileBuffer[FILENAME_MAX] = "/home/";
[2023-10-03 16:08:58] [build-stderr]           ^
[2023-10-03 16:08:58] [build-stderr] TaintedPath.c:22:1: warning: control reaches end of non-void function [-Wreturn-type]
[2023-10-03 16:08:58] [build-stderr]  }
[2023-10-03 16:08:58] [build-stderr]  ^
[2023-10-03 16:08:58] [build-stderr] make: [TaintedPath.o] Error 1 (ignored)

(c) database analysis

codeql database analyze \
/home/junseok/workdir/codeql-database  \
--format=csv \
--threads 4 \
--output=/home/junseok/workdir/cpp-ql-cwe-22-output \
/home/junseok/workdir/codeql/codeql-main-clone/cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql \
--verbosity=errors

(d) output of database analysis image

cat cpp-ql-cwe-22-output command shows empty results.

<< Questions >>

Why can't the ql file for CWE-022 detect vulnerabilities in c file for CWE-022? Is it not detected due to errors that exist during the build process?

mbg commented 1 year ago

Hi @ljs9904ljs 👋🏻

Thanks for your question. The C source file in that directory appears to be an outdated example file and as you suspect, you're likely not getting any results due to the build errors.

We have a working integration test based on similar code in https://github.com/github/codeql/blob/8a314dd2cf12adbb4c0061cee189f56ac6e4ed13/cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/

You can verify that this works as expected with codeql test run cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests. The test should pass, meaning that the output matches that found in the TaintedPath.expected file.

ljs9904ljs commented 1 year ago

Thank you!