Skip to content

fix: fixed security vulnerabilities from SAST/DAST scans #1

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

Closed
natanim-kemal wants to merge 1 commit into from
Closed

fix: fixed security vulnerabilities from SAST/DAST scans #1

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
2 changes: 1 addition & 1 deletion extension/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { WHITELIST } from './whitelist.js';

const API_URL = 'http://127.0.0.1:5000/predict';
const API_URL = 'https://127.0.0.1:5000/predict';
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Using HTTPS with 127.0.0.1 will require a valid SSL/TLS certificate for localhost. Without proper certificate configuration on the Flask server, this will fail with certificate verification errors. Consider either:

  1. Using HTTP for local development (127.0.0.1)
  2. Adding SSL context to Flask with a self-signed certificate and handling certificate validation in the extension
  3. Using 'localhost' instead of '127.0.0.1' if you have a localhost certificate
Suggested change
const API_URL = 'https://127.0.0.1:5000/predict';
const API_URL = 'http://127.0.0.1:5000/predict';

Copilot uses AI. Check for mistakes.
const CACHE_DURATION = 24 * 60 * 60 * 1000;

chrome.tabs.onUpdated.addListener((tabId, changeInfo, tab) => {
Expand Down
1 change: 0 additions & 1 deletion extension/popup.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

<head>
<title>Link Scanner</title>
<link href="https://api.fontshare.com/v2/css?f[]=clash-display@400,500,600,700&display=swap" rel="stylesheet">
<link rel="stylesheet" type="text/css" href="style.css">
</head>

Expand Down
3 changes: 2 additions & 1 deletion extension/popup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ function checkCurrentTab() {

urlDiv.textContent = url;

fetch('http://127.0.0.1:5000/predict', {
const API_URL = 'https://127.0.0.1:5000/predict';
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Using HTTPS with 127.0.0.1 will require a valid SSL/TLS certificate for localhost. Without proper certificate configuration on the Flask server, this will fail with certificate verification errors. Consider either:

  1. Using HTTP for local development (127.0.0.1)
  2. Adding SSL context to Flask with a self-signed certificate and handling certificate validation in the extension
  3. Using 'localhost' instead of '127.0.0.1' if you have a localhost certificate
Suggested change
const API_URL = 'https://127.0.0.1:5000/predict';
const API_URL = 'http://127.0.0.1:5000/predict';

Copilot uses AI. Check for mistakes.
fetch(API_URL, {
method: 'POST',
headers: {
'Content-Type': 'application/json'
Expand Down
97 changes: 60 additions & 37 deletions src/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,40 @@
from flask import Flask, request, jsonify
from flask import Flask, request, jsonify, make_response
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

Import of 'make_response' is not used.

Suggested change
from flask import Flask, request, jsonify, make_response
from flask import Flask, request, jsonify

Copilot uses AI. Check for mistakes.
from flask_cors import CORS
import joblib
import pandas as pd
from feature_extraction import extract_features, get_feature_names
import os
import sqlite3
import subprocess
import socket
import re

app = Flask(__name__)
CORS(app)

# ============================================================
# INTENTIONALLY VULNERABLE CODE FOR SECURITY ANALYSIS DEMO
# DO NOT USE IN PRODUCTION
# ============================================================

API_SECRET_KEY = "sk-prod-1234567890abcdef"
DATABASE_PASSWORD = "admin123"
ADMIN_TOKEN = "super_secret_token_12345"
CORS(app, origins=['http://localhost:3000', 'chrome-extension://*'])
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The CORS configuration allows 'http://localhost:3000' but the API is now expected to be served over HTTPS (as indicated by the extension changes). This creates a mismatch. If the API is served over HTTPS, the CORS origin should also be 'https://localhost:3000' or you should allow both HTTP and HTTPS origins for local development.

Suggested change
CORS(app, origins=['http://localhost:3000', 'chrome-extension://*'])
CORS(app, origins=['http://localhost:3000', 'https://localhost:3000', 'chrome-extension://*'])

Copilot uses AI. Check for mistakes.

MODEL_PATH = 'model.pkl'
SCALER_PATH = 'scaler.pkl'
DB_PATH = 'scan_logs.db'
LOGS_DIR = 'logs'

model = None
scaler = None


def add_security_headers(response):
response.headers['Content-Security-Policy'] = "default-src 'self'"
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The Content-Security-Policy header 'default-src self' is too restrictive for an API backend. This CSP is appropriate for web pages but will interfere with API responses. For a JSON API, either remove CSP entirely or use a more appropriate policy that doesn't restrict the API functionality.

Suggested change
response.headers['Content-Security-Policy'] = "default-src 'self'"

Copilot uses AI. Check for mistakes.
response.headers['X-Content-Type-Options'] = 'nosniff'
response.headers['X-Frame-Options'] = 'DENY'
response.headers['X-XSS-Protection'] = '1; mode=block'
response.headers['Permissions-Policy'] = 'geolocation=(), microphone=()'
return response


@app.after_request
def apply_security_headers(response):
return add_security_headers(response)


def init_db():
conn = sqlite3.connect(DB_PATH)
c = conn.cursor()
Expand Down Expand Up @@ -105,9 +113,7 @@ def predict():
})

except Exception as e:
import traceback
traceback.print_exc()
return jsonify({'error': str(e)}), 500
return jsonify({'error': 'An error occurred processing your request'}), 500


@app.route('/health', methods=['GET'])
Expand All @@ -128,11 +134,6 @@ def reload():
return jsonify({'success': success})


# ============================================================
# VULNERABLE ENDPOINTS FOR SECURITY ANALYSIS DEMONSTRATION
# DO NOT USE IN PRODUCTION
# ============================================================

@app.route('/log', methods=['POST'])
def log_scan():
data = request.get_json()
Expand All @@ -143,48 +144,70 @@ def log_scan():
conn = sqlite3.connect(DB_PATH)
c = conn.cursor()

query = f"INSERT INTO logs (url, result, timestamp) VALUES ('{url}', '{result}', datetime('now'))"
c.execute(query)
c.execute(
"INSERT INTO logs (url, result, timestamp) VALUES (?, ?, datetime('now'))",
(url, result)
)
Comment on lines +147 to +150
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The '/log' endpoint lacks input validation. The 'url' and 'result' parameters should be validated before being inserted into the database. Consider adding checks for:

  1. Maximum length limits to prevent excessively large data
  2. Content validation to ensure 'result' contains expected values (e.g., 'PHISHING' or 'LEGITIMATE')
  3. Basic format validation for the URL parameter

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +150
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The parameterized SQL query fix lacks test coverage. Consider adding tests to verify that:

  1. The endpoint correctly logs valid scan results
  2. The endpoint handles special characters in URL and result parameters
  3. SQL injection attempts are properly prevented

Copilot uses AI. Check for mistakes.

conn.commit()
conn.close()

return jsonify({'status': 'logged', 'url': url})


def is_valid_domain(domain):
pattern = r'^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z]{2,})+$'
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The domain validation regex pattern does not properly handle all valid domain formats. It requires at least two parts (e.g., 'example.com') and won't accept single-level domains like 'localhost' or IP addresses. This could break legitimate use cases. Consider whether single-level domains and IP addresses should be supported for the DNS lookup functionality.

Copilot uses AI. Check for mistakes.
return bool(re.match(pattern, domain))


@app.route('/lookup', methods=['GET'])
def dns_lookup():
domain = request.args.get('domain', '')

if not domain or not is_valid_domain(domain):
return jsonify({'error': 'Invalid domain format'}), 400

try:
result = subprocess.check_output(f"nslookup {domain}", shell=True, stderr=subprocess.STDOUT)
return jsonify({'result': result.decode('utf-8', errors='ignore')})
except subprocess.CalledProcessError as e:
return jsonify({'error': str(e)}), 500
ip_address = socket.gethostbyname(domain)
return jsonify({
'domain': domain,
'ip_address': ip_address
})
except socket.gaierror:
return jsonify({'error': 'Domain not found'}), 404
except Exception:
return jsonify({'error': 'Lookup failed'}), 500
Comment on lines +158 to +179
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The domain validation logic and socket-based DNS lookup lack test coverage. Consider adding tests to verify that:

  1. Valid domain formats are accepted
  2. Invalid domain formats (malicious patterns, command injection attempts) are rejected
  3. The DNS lookup correctly resolves valid domains
  4. Error handling works for non-existent domains

Copilot uses AI. Check for mistakes.


@app.route('/logs/<filename>')
def view_log(filename):
safe_filename = os.path.basename(filename)

if not safe_filename or '..' in filename or filename.startswith('/'):
Comment on lines +184 to +186
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The validation logic checks 'filename' for '..' and '/' after using 'os.path.basename(filename)' to create 'safe_filename'. Since basename removes directory components, checking the original 'filename' for these patterns after basename has already been applied is redundant. The checks on line 186 should use 'safe_filename' instead of 'filename' for consistency, or better yet, the checks should be performed before calling basename.

Suggested change
safe_filename = os.path.basename(filename)
if not safe_filename or '..' in filename or filename.startswith('/'):
if not filename or '..' in filename or filename.startswith('/'):
return jsonify({'error': 'Invalid filename'}), 400
safe_filename = os.path.basename(filename)
if not safe_filename:

Copilot uses AI. Check for mistakes.
return jsonify({'error': 'Invalid filename'}), 400

if not os.path.exists(LOGS_DIR):
os.makedirs(LOGS_DIR)

filepath = os.path.join(LOGS_DIR, safe_filename)

real_path = os.path.realpath(filepath)
logs_real_path = os.path.realpath(LOGS_DIR)

if not real_path.startswith(logs_real_path):
return jsonify({'error': 'Access denied'}), 403

try:
filepath = f"logs/{filename}"
with open(filepath, 'r') as f:
return f.read()
except FileNotFoundError:
return jsonify({'error': 'Log file not found'}), 404
except Exception as e:
return jsonify({'error': str(e)}), 500
except Exception:
return jsonify({'error': 'Unable to read file'}), 500
Comment on lines 182 to +206
Copy link

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

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

The path traversal protection in the log viewing endpoint lacks test coverage. Consider adding tests to verify that:

  1. Valid filenames are correctly accessed
  2. Path traversal attempts (e.g., '../../../etc/passwd') are blocked
  3. Absolute path attempts are rejected
  4. Files outside the LOGS_DIR are inaccessible

Copilot uses AI. Check for mistakes.


@app.route('/debug')
def debug_info():
return jsonify({
'api_key': API_SECRET_KEY,
'db_password': DATABASE_PASSWORD,
'model_path': MODEL_PATH,
'environment': dict(os.environ)
})


if __name__ == '__main__':
init_db()
app.run(host='127.0.0.1', port=5000, debug=True)
app.run(host='127.0.0.1', port=5000, debug=False)
Loading