Skip to content

Commit

Permalink
Fix all Checkmarx Medium/High vulns (#755)
Browse files Browse the repository at this point in the history
* Fix syncdb SQL vulnerability

* Fix path traversal docs vulnerability

* Add CRF protection to setting.py

* Add f-string suggestion to pylint ignore

We don't want to use f-strings anywhere as they create potential vulnerabilities

* Readd handler
  • Loading branch information
aloftus23 authored Jan 10, 2025
1 parent 2f321b2 commit ec145d8
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 15 deletions.
1 change: 1 addition & 0 deletions backend/.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ max-line-length=120

[MESSAGES CONTROL]
disable=C0103, # Conforms to snake_case
C0209, # Formatting a regular string which could be an f-string (consider-using-f-string)
C0301, # Line too long
C0415, # Import outside top-level (consider refactoring)
C0302, # Too many lines in module
Expand Down
15 changes: 10 additions & 5 deletions backend/src/xfd_django/xfd_api/tasks/syncdb_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def process_model(schema_editor: BaseDatabaseSchemaEditor, cursor, model):
table_name = model._meta.db_table

# Check if the table exists
cursor.execute(f"SELECT to_regclass('{table_name}');")
cursor.execute("SELECT to_regclass(%s);", [table_name])
table_exists = cursor.fetchone()[0] is not None

if table_exists:
Expand All @@ -275,7 +275,7 @@ def process_m2m_tables(schema_editor: BaseDatabaseSchemaEditor, cursor):
m2m_table_name = field.m2m_db_table()

# Check if the M2M table exists
cursor.execute(f"SELECT to_regclass('{m2m_table_name}');")
cursor.execute("SELECT to_regclass(%s);", [m2m_table_name])
table_exists = cursor.fetchone()[0] is not None

if not table_exists:
Expand Down Expand Up @@ -310,9 +310,14 @@ def update_table(schema_editor: BaseDatabaseSchemaEditor, model):
for column in extra_columns:
print(f"Removing extra column '{column}' from table '{table_name}'")
try:
cursor.execute(
f"ALTER TABLE {table_name} DROP COLUMN IF EXISTS {column};"
# Safely quote table and column names
safe_table_name = connection.ops.quote_name(table_name)
safe_column_name = connection.ops.quote_name(column)
# Construct and execute the query without f-strings
query = "ALTER TABLE {} DROP COLUMN IF EXISTS {};".format(
safe_table_name, safe_column_name
)
cursor.execute(query)
except Exception as e:
print(
f"Error dropping column '{column}' from table '{table_name}': {e}"
Expand All @@ -337,7 +342,7 @@ def cleanup_stale_tables(cursor):
for table in stale_tables:
print(f"Removing stale table: {table}")
try:
cursor.execute(f"DROP TABLE {table} CASCADE;")
cursor.execute("DROP TABLE %s CASCADE;", [connection.ops.quote_name(table)])
except Exception as e:
print(f"Error dropping stale table {table}: {e}")

Expand Down
13 changes: 13 additions & 0 deletions backend/src/xfd_django/xfd_django/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"django.contrib.auth.middleware.AuthenticationMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
"django.middleware.clickjacking.XFrameOptionsMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
]


Expand Down Expand Up @@ -123,3 +124,15 @@
PROJECT_NAME = "XFD Python API"

DJANGO_SETTINGS_MODULE = "xfd_django.settings"

# Ensure cookies are only sent over HTTPS
SESSION_COOKIE_SECURE = not IS_LOCAL # Only secure in production
CSRF_COOKIE_SECURE = not IS_LOCAL

# Prevent JavaScript access to cookies to mitigate XSS attacks
SESSION_COOKIE_HTTPONLY = True
CSRF_COOKIE_HTTPONLY = True

# SameSite policy to prevent CSRF via cross-origin requests
SESSION_COOKIE_SAMESITE = "Lax"
CSRF_COOKIE_SAMESITE = "Lax"
26 changes: 16 additions & 10 deletions frontend/scripts/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,27 @@ app.use(

// Fallback to index.html for client-side routing
app.get('*', (req, res) => {
const rootDir = path.resolve(__dirname, '../docs-build');
const staticFilePath = path.resolve(rootDir, '.' + req.path);
const rootDir = path.resolve(__dirname, '../docs-build'); // Define the root directory
const requestedPath = path.join(rootDir, req.path); // Join the requested path with rootDir
const resolvedPath = path.resolve(requestedPath); // Resolve to an absolute path

// Check that the file path is under the root directory
if (!staticFilePath.startsWith(rootDir)) {
// Ensure the resolved path is within the rootDir
if (!resolvedPath.startsWith(rootDir)) {
res.status(403).send('Forbidden');
return;
}

// Serve the file if it exists
if (fs.existsSync(staticFilePath) && fs.lstatSync(staticFilePath).isFile()) {
res.sendFile(staticFilePath);
} else {
// Fallback to index.html for client-side routing
res.sendFile(path.join(rootDir, 'index.html'));
// Check if the file exists and is a valid file
try {
if (fs.existsSync(resolvedPath) && fs.lstatSync(resolvedPath).isFile()) {
res.sendFile(resolvedPath); // Serve the file
} else {
// Fallback to index.html for client-side routing
res.sendFile(path.join(rootDir, 'index.html'));
}
} catch (error) {
console.error('Error while serving file:', error);
res.status(500).send('Internal Server Error');
}
});

Expand Down

0 comments on commit ec145d8

Please sign in to comment.