From 2ade7eb52792ef481aaf711dc07029629ea107d9 Mon Sep 17 00:00:00 2001
From: Eli Bosley <ekbosley@gmail.com>
Date: Wed, 14 May 2025 20:21:52 -0400
Subject: [PATCH] feat: initial codeql setup (#1390)

---
 .github/codeql/README.md                      |  49 +++++++
 .github/codeql/codeql-config.yml              |  16 +++
 .../javascript/api-auth-bypass.ql             |  45 ++++++
 .../javascript/graphql-injection.ql           |  77 +++++++++++
 .../javascript/hardcoded-secrets.ql           |  53 +++++++
 .../javascript/insecure-crypto.ql             |  90 ++++++++++++
 .../javascript/path-traversal.ql              | 130 ++++++++++++++++++
 .github/workflows/codeql-analysis.yml         |  40 ++++++
 8 files changed, 500 insertions(+)
 create mode 100644 .github/codeql/README.md
 create mode 100644 .github/codeql/codeql-config.yml
 create mode 100644 .github/codeql/custom-queries/javascript/api-auth-bypass.ql
 create mode 100644 .github/codeql/custom-queries/javascript/graphql-injection.ql
 create mode 100644 .github/codeql/custom-queries/javascript/hardcoded-secrets.ql
 create mode 100644 .github/codeql/custom-queries/javascript/insecure-crypto.ql
 create mode 100644 .github/codeql/custom-queries/javascript/path-traversal.ql
 create mode 100644 .github/workflows/codeql-analysis.yml

diff --git a/.github/codeql/README.md b/.github/codeql/README.md
new file mode 100644
index 000000000..32d3c3d37
--- /dev/null
+++ b/.github/codeql/README.md
@@ -0,0 +1,49 @@
+# CodeQL Security Analysis for Unraid API
+
+This directory contains custom CodeQL queries and configurations for security analysis of the Unraid API codebase.
+
+## Overview
+
+The analysis is configured to run:
+- On all pushes to the main branch
+- On all pull requests
+- Weekly via scheduled runs
+
+## Custom Queries
+
+The following custom queries are implemented:
+
+1. **API Authorization Bypass Detection**  
+   Identifies API handlers that may not properly check authorization before performing operations.
+
+2. **GraphQL Injection Detection**  
+   Detects potential injection vulnerabilities in GraphQL queries and operations.
+
+3. **Hardcoded Secrets Detection**  
+   Finds potential hardcoded secrets or credentials in the codebase.
+
+4. **Insecure Cryptographic Implementations**  
+   Identifies usage of weak cryptographic algorithms or insecure random number generation.
+
+5. **Path Traversal Vulnerability Detection**  
+   Detects potential path traversal vulnerabilities in file system operations.
+
+## Configuration
+
+The CodeQL analysis is configured in:
+- `.github/workflows/codeql-analysis.yml` - Workflow configuration
+- `.github/codeql/codeql-config.yml` - CodeQL engine configuration
+
+## Running Locally
+
+To run these queries locally:
+
+1. Install the CodeQL CLI: https://github.com/github/codeql-cli-binaries/releases
+2. Create a CodeQL database:
+   ```
+   codeql database create <db-name> --language=javascript --source-root=.
+   ```
+3. Run a query:
+   ```
+   codeql query run .github/codeql/custom-queries/javascript/api-auth-bypass.ql --database=<db-name>
+   ``` 
\ No newline at end of file
diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml
new file mode 100644
index 000000000..a787df1a6
--- /dev/null
+++ b/.github/codeql/codeql-config.yml
@@ -0,0 +1,16 @@
+name: "Unraid API CodeQL Configuration"
+
+disable-default-queries: false
+
+queries:
+  - name: Extended Security Queries
+    uses: security-extended
+  - name: Custom Unraid API Queries
+    uses: ./.github/codeql/custom-queries
+
+query-filters:
+  - exclude:
+      problem.severity:
+        - warning
+        - recommendation
+      tags contain: security 
\ No newline at end of file
diff --git a/.github/codeql/custom-queries/javascript/api-auth-bypass.ql b/.github/codeql/custom-queries/javascript/api-auth-bypass.ql
new file mode 100644
index 000000000..e16437123
--- /dev/null
+++ b/.github/codeql/custom-queries/javascript/api-auth-bypass.ql
@@ -0,0 +1,45 @@
+/**
+ * @name Potential API Authorization Bypass
+ * @description Functions that process API requests without verifying authorization may lead to security vulnerabilities.
+ * @kind problem
+ * @problem.severity error
+ * @precision medium
+ * @id js/api-auth-bypass
+ * @tags security
+ *       external/cwe/cwe-285
+ */
+
+import javascript
+
+/**
+ * Identifies functions that appear to handle API requests
+ */
+predicate isApiHandler(Function f) {
+  exists(f.getAParameter()) and
+  (
+    f.getName().regexpMatch("(?i).*(api|handler|controller|resolver|endpoint).*") or
+    exists(CallExpr call |
+      call.getCalleeName().regexpMatch("(?i).*(get|post|put|delete|patch).*") and
+      call.getArgument(1) = f
+    )
+  )
+}
+
+/**
+ * Identifies expressions that appear to perform authorization checks
+ */
+predicate isAuthCheck(DataFlow::Node node) {
+  exists(CallExpr call |
+    call.getCalleeName().regexpMatch("(?i).*(authorize|authenticate|isAuth|checkAuth|verifyAuth|hasPermission|isAdmin|canAccess).*") and
+    call.flow().getASuccessor*() = node
+  )
+}
+
+from Function apiHandler
+where
+  isApiHandler(apiHandler) and
+  not exists(DataFlow::Node authCheck | 
+    isAuthCheck(authCheck) and
+    authCheck.getEnclosingExpr().getEnclosingFunction() = apiHandler
+  )
+select apiHandler, "API handler function may not perform proper authorization checks." 
\ No newline at end of file
diff --git a/.github/codeql/custom-queries/javascript/graphql-injection.ql b/.github/codeql/custom-queries/javascript/graphql-injection.ql
new file mode 100644
index 000000000..ac43104a0
--- /dev/null
+++ b/.github/codeql/custom-queries/javascript/graphql-injection.ql
@@ -0,0 +1,77 @@
+/**
+ * @name Potential GraphQL Injection
+ * @description User-controlled input used directly in GraphQL queries may lead to injection vulnerabilities.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id js/graphql-injection
+ * @tags security
+ *       external/cwe/cwe-943
+ */
+
+import javascript
+import DataFlow::PathGraph
+
+class GraphQLQueryExecution extends DataFlow::CallNode {
+  GraphQLQueryExecution() {
+    exists(string name |
+      name = this.getCalleeName() and
+      (
+        name = "execute" or 
+        name = "executeQuery" or
+        name = "query" or
+        name.regexpMatch("(?i).*graphql.*query.*")
+      )
+    )
+  }
+
+  DataFlow::Node getQuery() {
+    result = this.getArgument(0)
+  }
+}
+
+class UserControlledInput extends DataFlow::Node {
+  UserControlledInput() {
+    exists(DataFlow::ParameterNode param | 
+      param.getName().regexpMatch("(?i).*(query|request|input|args|variables|params).*") and
+      this = param
+    )
+    or
+    exists(DataFlow::PropRead prop |
+      prop.getPropertyName().regexpMatch("(?i).*(query|request|input|args|variables|params).*") and
+      this = prop
+    )
+  }
+}
+
+/**
+ * Holds if `node` is a string concatenation.
+ */
+predicate isStringConcatenation(DataFlow::Node node) {
+  exists(BinaryExpr concat |
+    concat.getOperator() = "+" and
+    concat.flow() = node
+  )
+}
+
+class GraphQLInjectionConfig extends TaintTracking::Configuration {
+  GraphQLInjectionConfig() { this = "GraphQLInjectionConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source instanceof UserControlledInput
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(GraphQLQueryExecution exec | sink = exec.getQuery())
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    // Add any GraphQL-specific taint steps if needed
+    isStringConcatenation(succ) and
+    succ.(DataFlow::BinaryExprNode).getAnOperand() = pred
+  }
+}
+
+from GraphQLInjectionConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "GraphQL query may contain user-controlled input from $@.", source.getNode(), "user input" 
\ No newline at end of file
diff --git a/.github/codeql/custom-queries/javascript/hardcoded-secrets.ql b/.github/codeql/custom-queries/javascript/hardcoded-secrets.ql
new file mode 100644
index 000000000..c8c238d7a
--- /dev/null
+++ b/.github/codeql/custom-queries/javascript/hardcoded-secrets.ql
@@ -0,0 +1,53 @@
+/**
+ * @name Hardcoded Secrets
+ * @description Hardcoded secrets or credentials in source code can lead to security vulnerabilities.
+ * @kind problem
+ * @problem.severity error
+ * @precision medium
+ * @id js/hardcoded-secrets
+ * @tags security
+ *       external/cwe/cwe-798
+ */
+
+import javascript
+
+/**
+ * Identifies variable declarations or assignments that may contain secrets
+ */
+predicate isSensitiveAssignment(DataFlow::Node node) {
+  exists(DataFlow::PropWrite propWrite |
+    propWrite.getPropertyName().regexpMatch("(?i).*(secret|key|password|token|credential|auth).*") and
+    propWrite.getRhs() = node
+  )
+  or
+  exists(VariableDeclarator decl |
+    decl.getName().regexpMatch("(?i).*(secret|key|password|token|credential|auth).*") and
+    decl.getInit().flow() = node
+  )
+}
+
+/**
+ * Identifies literals that look like secrets
+ */
+predicate isSecretLiteral(StringLiteral literal) {
+  // Match alphanumeric strings of moderate length that may be secrets
+  literal.getValue().regexpMatch("[A-Za-z0-9_\\-]{8,}") and
+  
+  not (
+    // Skip likely non-sensitive literals
+    literal.getValue().regexpMatch("(?i)^(true|false|null|undefined|localhost|development|production|staging)$") or
+    // Skip URLs without credentials
+    literal.getValue().regexpMatch("^https?://[^:@/]+")
+  )
+}
+
+from DataFlow::Node source
+where
+  isSensitiveAssignment(source) and
+  (
+    exists(StringLiteral literal | 
+      literal.flow() = source and 
+      isSecretLiteral(literal)
+    )
+  )
+select source, "This assignment may contain a hardcoded secret or credential." 
\ No newline at end of file
diff --git a/.github/codeql/custom-queries/javascript/insecure-crypto.ql b/.github/codeql/custom-queries/javascript/insecure-crypto.ql
new file mode 100644
index 000000000..0c29cd531
--- /dev/null
+++ b/.github/codeql/custom-queries/javascript/insecure-crypto.ql
@@ -0,0 +1,90 @@
+/**
+ * @name Insecure Cryptographic Implementation
+ * @description Usage of weak cryptographic algorithms or improper implementations can lead to security vulnerabilities.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @id js/insecure-crypto
+ * @tags security
+ *       external/cwe/cwe-327
+ */
+
+import javascript
+
+/**
+ * Identifies calls to crypto functions with insecure algorithms
+ */
+predicate isInsecureCryptoCall(CallExpr call) {
+  // Node.js crypto module uses
+  exists(string methodName |
+    methodName = call.getCalleeName() and
+    (
+      // Detect MD5 usage
+      methodName.regexpMatch("(?i).*md5.*") or
+      methodName.regexpMatch("(?i).*sha1.*") or
+      
+      // Insecure crypto constructors
+      (
+        methodName = "createHash" or
+        methodName = "createCipheriv" or
+        methodName = "createDecipher"
+      ) and
+      (
+        exists(StringLiteral algo | 
+          algo = call.getArgument(0) and
+          (
+            algo.getValue().regexpMatch("(?i).*(md5|md4|md2|sha1|des|rc4|blowfish).*") or
+            algo.getValue().regexpMatch("(?i).*(ecb).*") // ECB mode
+          )
+        )
+      )
+    )
+  )
+  or
+  // Browser crypto API uses
+  exists(MethodCallExpr mce, string propertyName |
+    propertyName = mce.getMethodName() and
+    (
+      propertyName = "subtle" and
+      exists(MethodCallExpr subtleCall | 
+        subtleCall.getReceiver() = mce and 
+        subtleCall.getMethodName() = "encrypt" and
+        exists(ObjectExpr obj | 
+          obj = subtleCall.getArgument(0) and
+          exists(Property p | 
+            p = obj.getAProperty() and
+            p.getName() = "name" and
+            exists(StringLiteral algo | 
+              algo = p.getInit() and
+              algo.getValue().regexpMatch("(?i).*(rc4|des|aes-cbc).*")
+            )
+          )
+        )
+      )
+    )
+  )
+}
+
+/**
+ * Identifies usage of Math.random() for security-sensitive operations
+ */
+predicate isInsecureRandomCall(CallExpr call) {
+  exists(PropertyAccess prop | 
+    prop.getPropertyName() = "random" and
+    prop.getBase().toString() = "Math" and
+    call.getCallee() = prop
+  )
+}
+
+from Expr insecureExpr, string message
+where
+  (
+    insecureExpr instanceof CallExpr and
+    isInsecureCryptoCall(insecureExpr) and
+    message = "Using potentially insecure cryptographic algorithm or mode."
+  ) or (
+    insecureExpr instanceof CallExpr and
+    isInsecureRandomCall(insecureExpr) and
+    message = "Using Math.random() for security-sensitive operation. Consider using crypto.getRandomValues() instead."
+  )
+select insecureExpr, message 
\ No newline at end of file
diff --git a/.github/codeql/custom-queries/javascript/path-traversal.ql b/.github/codeql/custom-queries/javascript/path-traversal.ql
new file mode 100644
index 000000000..108b0ad5a
--- /dev/null
+++ b/.github/codeql/custom-queries/javascript/path-traversal.ql
@@ -0,0 +1,130 @@
+/**
+ * @name Path Traversal Vulnerability
+ * @description User-controlled inputs used in file operations may allow for path traversal attacks.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id js/path-traversal
+ * @tags security
+ *       external/cwe/cwe-22
+ */
+
+import javascript
+import DataFlow::PathGraph
+
+/**
+ * Identifies sources of user-controlled input
+ */
+class UserInput extends DataFlow::Node {
+  UserInput() {
+    // HTTP request parameters
+    exists(DataFlow::ParameterNode param |
+      param.getName().regexpMatch("(?i).*(req|request|param|query|body|user|input).*") and
+      this = param
+    )
+    or
+    // Access to common request properties
+    exists(DataFlow::PropRead prop |
+      (
+        prop.getPropertyName() = "query" or
+        prop.getPropertyName() = "body" or
+        prop.getPropertyName() = "params" or
+        prop.getPropertyName() = "files"
+      ) and
+      this = prop
+    )
+  }
+}
+
+/**
+ * Identifies fs module imports
+ */
+class FileSystemAccess extends DataFlow::CallNode {
+  FileSystemAccess() {
+    // Node.js fs module functions
+    exists(string name |
+      name = this.getCalleeName() and
+      (
+        name = "readFile" or
+        name = "readFileSync" or
+        name = "writeFile" or
+        name = "writeFileSync" or
+        name = "appendFile" or
+        name = "appendFileSync" or
+        name = "createReadStream" or
+        name = "createWriteStream" or
+        name = "openSync" or
+        name = "open"
+      )
+    )
+    or
+    // File system operations via require('fs')
+    exists(DataFlow::SourceNode fsModule, string methodName |
+      (fsModule.getAPropertyRead("promises") or fsModule).flowsTo(this.getReceiver()) and
+      methodName = this.getMethodName() and
+      (
+        methodName = "readFile" or
+        methodName = "writeFile" or
+        methodName = "appendFile" or
+        methodName = "readdir" or
+        methodName = "stat"
+      )
+    )
+  }
+
+  DataFlow::Node getPathArgument() {
+    result = this.getArgument(0)
+  }
+}
+
+/**
+ * Identifies sanitization of file paths
+ */
+predicate isPathSanitized(DataFlow::Node node) {
+  // Check for path normalization or validation
+  exists(DataFlow::CallNode call |
+    (
+      call.getCalleeName() = "resolve" or
+      call.getCalleeName() = "normalize" or
+      call.getCalleeName() = "isAbsolute" or
+      call.getCalleeName() = "relative" or
+      call.getCalleeName().regexpMatch("(?i).*(sanitize|validate|check).*path.*")
+    ) and
+    call.flowsTo(node)
+  )
+  or
+  // Check for path traversal mitigation patterns
+  exists(DataFlow::CallNode call |
+    call.getCalleeName() = "replace" and
+    exists(StringLiteral regex |
+      regex = call.getArgument(0).(DataFlow::RegExpCreationNode).getSource().getAChildExpr() and
+      regex.getValue().regexpMatch("(\\.\\./|\\.\\.\\\\)")
+    ) and
+    call.flowsTo(node)
+  )
+}
+
+/**
+ * Configuration for tracking flow from user input to file system operations
+ */
+class PathTraversalConfig extends TaintTracking::Configuration {
+  PathTraversalConfig() { this = "PathTraversalConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source instanceof UserInput
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(FileSystemAccess fileAccess |
+      sink = fileAccess.getPathArgument()
+    )
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    isPathSanitized(node)
+  }
+}
+
+from PathTraversalConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "File system operation depends on a user-controlled value $@.", source.getNode(), "user input" 
\ No newline at end of file
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
new file mode 100644
index 000000000..e64322310
--- /dev/null
+++ b/.github/workflows/codeql-analysis.yml
@@ -0,0 +1,40 @@
+name: "CodeQL Security Analysis"
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+  schedule:
+    - cron: '0 0 * * 0'  # Run weekly on Sundays
+
+jobs:
+  analyze:
+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    strategy:
+      fail-fast: false
+      matrix:
+        language: [ 'javascript', 'typescript' ]
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v2
+      with:
+        languages: ${{ matrix.language }}
+        config-file: ./.github/codeql/codeql-config.yml
+        queries: +security-and-quality
+
+    - name: Autobuild
+      uses: github/codeql-action/autobuild@v2
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v2 
\ No newline at end of file