Skip to content

chore: New schema change for vulnerabilities test #37204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sagar-qa007 merged 1 commit into from
Nov 5, 2024
Merged

chore: New schema change for vulnerabilities test #37204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 0 additions & 79 deletions .github/workflows/test-vulnerabilities-data.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,85 +29,6 @@ jobs:
- name: Install pg
run: npm install pg

- name: Fetch vulnerability data
id: vulnerability_data
env:
DB_HOST: ${{ secrets.CYPRESS_DB_HOST }}
DB_NAME: ${{ secrets.CYPRESS_DB_NAME }}
DB_USER: ${{ secrets.CYPRESS_DB_USER }}
DB_PWD: ${{ secrets.CYPRESS_DB_PWD }}
uses: actions/github-script@v7
with:
script: |
const { Pool } = require("pg");
const fs = require('fs');
const path = require('path');
const { DB_HOST, DB_NAME, DB_USER, DB_PWD } = process.env;

const pool = new Pool({
user: DB_USER,
host: DB_HOST,
database: DB_NAME,
password: DB_PWD,
port: 5432,
connectionTimeoutMillis: 60000,
});

(async () => {
const client = await pool.connect();
try {
// Fetch vurn_id, product, scanner_tool, and priority from the database
const result = await client.query(`SELECT vurn_id, product, scanner_tool, priority FROM vulnerability_tracking`);
console.log('Vulnerability Data:', result.rows);

// Extract relevant fields from the result
const extractedData = result.rows.map(({ vurn_id, product, scanner_tool, priority }) => ({
vurn_id,
product,
scanner_tool,
priority
}));
console.log('Extracted Vulnerability Data:', extractedData);

// Prepare CSV content
const csvContent = [
['vurn_id', 'product', 'scanner_tool', 'priority'], // Add priority column header
...extractedData.map(row => [row.vurn_id, row.product, row.scanner_tool, row.priority])
]
.map(e => e.join(',')) // Join columns
.join('\n'); // Join rows

// Write to CSV file in workspace
const csvFilePath = path.join(process.env.GITHUB_WORKSPACE, 'vulnerability_base_data.csv');
fs.writeFileSync(csvFilePath, csvContent);
console.log(`Data successfully written to ${csvFilePath}`);

// Prepare TXT content
const txtContent = extractedData
.map(row => `vurn_id: ${row.vurn_id}, product: ${row.product}, scanner_tool: ${row.scanner_tool}, priority: ${row.priority}`)
.join('\n'); // Join rows

// Write to TXT file in workspace
const txtFilePath = path.join(process.env.GITHUB_WORKSPACE, 'vulnerability_base_data.txt');
fs.writeFileSync(txtFilePath, txtContent);
console.log(`Data successfully written to ${txtFilePath}`);

client.release();
return extractedData; // Return the extracted data
} catch (err) {
console.error('Error fetching vulnerability data:', err);
client.release();
}
})();

- name: Upload Vulnerability Data
uses: actions/upload-artifact@v3
with:
name: vulnerability-data
path: |
vulnerability_base_data.csv
vulnerability_base_data.txt

# Run Scout vulnerability data script
- name: Run Scout vulnerability data script
if: always()
Expand Down
103 changes: 44 additions & 59 deletions scripts/scout_vulnerabilities_data.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash

#Check required environment variables
# Check required environment variables
required_vars=("DB_HOST" "DB_NAME" "DB_USER" "DB_PWD")
for var in "${required_vars[@]}"; do
if [ -z "${!var}" ] || [[ "${!var}" == "your_${var,,}" ]]; then
Expand All @@ -25,7 +25,7 @@ OLD_VULN_FILE="${5:-vulnerability_base_data.csv}"
install_docker_scout() {
echo "Installing Docker Scout..."
local attempts=0
while [ $attempts -lt 3 ]; do
while [ $attempts -lt 5 ]; do
echo "Attempt $((attempts + 1))..."
curl -fsSL https://raw.githubusercontent.com/docker/scout-cli/main/install.sh -o install-scout.sh
sh install-scout.sh &> install_scout_log.txt
Expand Down Expand Up @@ -75,98 +75,83 @@ docker scout cves "$IMAGE" | grep -E "✗ |CVE-" | awk -v product_name="$product
{
# Check for valid vulnerability data and format it correctly
if ($2 != "" && $3 ~ /^CVE-/) {
# Extract severity level, CVE ID and format output correctly
# Extract severity level, CVE ID, and format output correctly
Comment on lines 77 to +78
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Prevent CSV injection in vulnerability data

The current implementation is vulnerable to CSV injection. Add proper escaping:

-        print $3","product_name",""SCOUT"","$2
+        gsub(/[,"]/, "\\\\&", $3)
+        gsub(/[,"]/, "\\\\&", $2)
+        print $3 "," product_name ",\"SCOUT\"," $2

Committable suggestion skipped: line range outside the PR's diff.

print $3","product_name",""SCOUT"","$2
}
}' | sort -u > "$CSV_OUTPUT_FILE"

# Check if the CSV output file is empty
[ -s "$CSV_OUTPUT_FILE" ] || echo "No vulnerabilities found for image: $IMAGE" > "$CSV_OUTPUT_FILE"

# Compare new vulnerabilities against old vulnerabilities
echo "Comparing new vulnerabilities with existing vulnerabilities in $OLD_VULN_FILE..."
if [ -s "$OLD_VULN_FILE" ]; then
awk -F, 'NR==FNR {seen[$1","$2","$3","$4]; next} !($1","$2","$3","$4 in seen)' "$OLD_VULN_FILE" "$CSV_OUTPUT_FILE" > "scout_vulnerabilities_diff.csv"
else
echo "$OLD_VULN_FILE is empty. All new vulnerabilities will be included."
cp "$CSV_OUTPUT_FILE" "scout_vulnerabilities_diff.csv"
fi

# Output for verification
echo "Fetching passed data..."
cat "$OLD_VULN_FILE"
echo ""
echo "Fetching new data..."
cat "$CSV_OUTPUT_FILE"
echo ""
echo "Fetching diff..."
cat "scout_vulnerabilities_diff.csv"
echo ""

# Insert new vulnerabilities into the PostgreSQL database using psql
insert_vulns_into_db() {
local count=0
local query_file="insert_vulns.sql"
echo "BEGIN;" > "$query_file" # Start the transaction

# Create an associative array to hold existing entries from the database
declare -A existing_entries

# Fetch existing vulnerabilities from the database to avoid duplicates
psql -t -c "SELECT vurn_id, product, scanner_tool, priority FROM vulnerability_tracking WHERE scanner_tool = 'SCOUT'" "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" | while IFS='|' read -r db_vurn_id db_product db_scanner_tool db_priority; do
existing_entries["$db_product,$db_scanner_tool,$db_vurn_id"]="$db_priority"
done
echo "BEGIN;" > "$query_file"

while IFS=, read -r vurn_id product scanner_tool priority; do
# Skip empty lines
if [[ -z "$vurn_id" || -z "$priority" || -z "$product" || -z "$scanner_tool" ]]; then
echo "Skipping empty vulnerability entry"
continue
fi

# Check if the entry already exists
if [[ -n "${existing_entries["$product,$scanner_tool,$vurn_id"]}" ]]; then
echo "Entry for $vurn_id already exists in the database. Skipping."
continue
fi

local pr_id="$GITHUB_PR_ID"
local pr_link="$GITHUB_PR_LINK"
local pr_id="${GITHUB_PR_ID:-}"
local pr_link="${GITHUB_PR_LINK:-}"
local created_date=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
local update_date="$created_date"
local comments="Initial vulnerability report"
local owner="John Doe"
local pod="Security"

# Escape single quotes in vulnerability ID, product, and priority
# Clean up input values
vurn_id=$(echo "$vurn_id" | sed "s/'/''/g")
priority=$(echo "$priority" | sed "s/'/''/g")
product=$(echo "$product" | sed "s/'/''/g")
scanner_tool=$(echo "$scanner_tool" | sed "s/'/''/g")
product=$(echo "$product" | sed "s/'/''/g" | tr -d '[:space:]' | sed 's/[|]//g' | sed 's/,$//')
scanner_tool=$(echo "$scanner_tool" | sed "s/'/''/g" | tr -d '[:space:]' | sed 's/[|]//g' | sed 's/,$//')

# Write each insert query to the SQL file
echo "INSERT INTO vulnerability_tracking (product, scanner_tool, vurn_id, priority, pr_id, pr_link, github_run_id, created_date, update_date, comments, owner, pod) VALUES ('$product', '$scanner_tool', '$vurn_id', '$priority', '$pr_id', '$pr_link', '$GITHUB_RUN_ID', '$created_date', '$update_date', '$comments', '$owner', '$pod');" >> "$query_file"
# Fetch existing values for this vulnerability ID
existing_entry=$(psql -t -c "SELECT product, scanner_tool FROM vulnerability_tracking WHERE vurn_id = '$vurn_id'" "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" 2>/dev/null)

((count++))
done < "scout_vulnerabilities_diff.csv"
# Process fetched data
if [[ -z "$existing_entry" ]]; then
combined_products="$product"
combined_scanner_tools="$scanner_tool"
else
IFS='|' read -r existing_product existing_scanner_tool <<< "$existing_entry"
combined_products=$(echo "$existing_product,$product" | tr ',' '\n' | sed '/^$/d' | sort -u | tr '\n' ',' | sed 's/^,//; s/,$//')
combined_scanner_tools=$(echo "$existing_scanner_tool,$scanner_tool" | tr ',' '\n' | sed '/^$/d' | sort -u | tr '\n' ',' | sed 's/^,//; s/,$//')
fi

echo "COMMIT;" >> "$query_file" # End the transaction
# Write the insert query to the SQL file
echo "INSERT INTO vulnerability_tracking (product, scanner_tool, vurn_id, priority, pr_id, pr_link, github_run_id, created_date, update_date, comments, owner, pod)
VALUES ('$combined_products', '$combined_scanner_tools', '$vurn_id', '$priority', '$pr_id', '$pr_link', '$GITHUB_RUN_ID', '$created_date', '$created_date', '$comments', '$owner', '$pod')
ON CONFLICT (vurn_id)
DO UPDATE SET
product = '$combined_products',
scanner_tool = '$combined_scanner_tools',
priority = EXCLUDED.priority,
pr_id = EXCLUDED.pr_id,
pr_link = EXCLUDED.pr_link,
github_run_id = EXCLUDED.github_run_id,
update_date = EXCLUDED.update_date,
comments = EXCLUDED.comments,
owner = EXCLUDED.owner,
pod = EXCLUDED.pod;" >> "$query_file"
Comment on lines +110 to +137
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Critical: SQL Injection vulnerabilities confirmed in vulnerability tracking scripts

The scripts are vulnerable to SQL injection through direct variable interpolation in SQL queries. Key issues:

  • Direct variable interpolation in SELECT queries: WHERE vurn_id = '$vurn_id'
  • Unescaped variables in INSERT/UPDATE statements
  • Basic string cleaning (sed "s/'/''/g") is insufficient for SQL injection prevention

Fix by using psql's built-in parameterization:

- existing_entry=$(psql -t -c "SELECT product, scanner_tool FROM vulnerability_tracking WHERE vurn_id = '$vurn_id'" ...)
+ existing_entry=$(psql -t -c "SELECT product, scanner_tool FROM vulnerability_tracking WHERE vurn_id = :vurn_id" -v vurn_id="$vurn_id" ...)

For the INSERT/UPDATE queries, use a prepared statement file with parameters:

\set ON_ERROR_STOP on
PREPARE vuln_insert AS
INSERT INTO vulnerability_tracking (...) VALUES (:products, :scanner_tools, ...) 
ON CONFLICT (vurn_id) DO UPDATE SET ...;
🔗 Analysis chain

Critical: Fix SQL injection vulnerabilities

The current implementation is vulnerable to SQL injection attacks. Use prepared statements:

Replace direct variable interpolation with prepared statements:

-    existing_entry=$(psql -t -c "SELECT product, scanner_tool FROM vulnerability_tracking WHERE vurn_id = '$vurn_id'" "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" 2>/dev/null)
+    existing_entry=$(psql -t -c "SELECT product, scanner_tool FROM vulnerability_tracking WHERE vurn_id = $1" -v vurn_id="$vurn_id" "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" 2>/dev/null)

These commands will help us:

  1. Find SQL queries with variable interpolation
  2. Analyze the complete script context
  3. Identify other database connection patterns

This will give us a better understanding of the SQL injection concerns and database interaction patterns in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify potential SQL injection points
rg -l "psql.*\$[a-zA-Z_]" .

Length of output: 28

Script:

#!/bin/bash
# Search for SQL queries in the script with a more specific pattern
rg -l "psql.*-c.*\$" .

Length of output: 233

Script:

#!/bin/bash
# Get the content of the script to analyze SQL usage
cat scripts/scout_vulnerabilities_data.sh

Length of output: 5818

Script:

#!/bin/bash
# Look for other database interaction patterns
rg "postgresql://" .

Length of output: 5956


done < "$CSV_OUTPUT_FILE"

echo "COMMIT;" >> "$query_file"
echo "Queries written to $query_file."

# Execute the SQL file
psql -e "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" -f "$query_file"

# Check if the execution was successful
if [ $? -eq 0 ]; then
# Execute the SQL file and rollback on failure
if psql -e "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" -f "$query_file"; then
echo "Vulnerabilities successfully inserted into the database."
else
echo "Error: Failed to insert vulnerabilities. Please check the database connection or query."
echo "Error: Failed to insert vulnerabilities. Performing rollback."
echo "ROLLBACK;" | psql "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME"
exit 1
Comment on lines +144 to 150
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling and cleanup

Enhance error handling with detailed logging and cleanup:

   if psql -e "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" -f "$query_file"; then
     echo "Vulnerabilities successfully inserted into the database."
+    rm -f "$query_file"
   else
-    echo "Error: Failed to insert vulnerabilities. Performing rollback."
+    echo "Error: Failed to insert vulnerabilities. Check postgresql logs. Performing rollback."
     echo "ROLLBACK;" | psql "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME"
+    rm -f "$query_file"
     exit 1
   fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Execute the SQL file and rollback on failure
if psql -e "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" -f "$query_file"; then
echo "Vulnerabilities successfully inserted into the database."
else
echo "Error: Failed to insert vulnerabilities. Please check the database connection or query."
echo "Error: Failed to insert vulnerabilities. Performing rollback."
echo "ROLLBACK;" | psql "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME"
exit 1
# Execute the SQL file and rollback on failure
if psql -e "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME" -f "$query_file"; then
echo "Vulnerabilities successfully inserted into the database."
rm -f "$query_file"
else
echo "Error: Failed to insert vulnerabilities. Check postgresql logs. Performing rollback."
echo "ROLLBACK;" | psql "postgresql://$DB_USER:$DB_PWD@$DB_HOST/$DB_NAME"
rm -f "$query_file"
exit 1

fi
}

# Call the function to generate the insert queries and execute them
if [ -s "scout_vulnerabilities_diff.csv" ]; then
if [ -s "$CSV_OUTPUT_FILE" ]; then
insert_vulns_into_db
else
echo "No new vulnerabilities to insert."
Expand Down
Loading