Fix all high and medium severity security issues found by Bandit
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
This commit is contained in:
+6
-4
@@ -53,7 +53,7 @@ async def exchange_dropbox_token(
|
||||
|
||||
# Make the token request
|
||||
logger.info("Sending POST request to Dropbox for token exchange")
|
||||
response = requests.post(token_url, data=payload)
|
||||
response = requests.post(token_url, data=payload, timeout=settings.http_request_timeout)
|
||||
|
||||
# Check if the request was successful
|
||||
logger.info(f"Token exchange response status: {response.status_code}")
|
||||
@@ -176,7 +176,8 @@ async def test_dropbox_token(request: Request):
|
||||
headers = {"Authorization": f"Bearer {settings.dropbox_refresh_token}"}
|
||||
response = requests.post(
|
||||
"https://api.dropboxapi.com/2/users/get_current_account",
|
||||
headers=headers
|
||||
headers=headers,
|
||||
timeout=settings.http_request_timeout
|
||||
)
|
||||
|
||||
# If token is invalid, try refreshing it
|
||||
@@ -192,7 +193,7 @@ async def test_dropbox_token(request: Request):
|
||||
"client_secret": settings.dropbox_app_secret
|
||||
}
|
||||
|
||||
refresh_response = requests.post(refresh_url, data=refresh_data)
|
||||
refresh_response = requests.post(refresh_url, data=refresh_data, timeout=settings.http_request_timeout)
|
||||
|
||||
if refresh_response.status_code != 200:
|
||||
logger.error(f"Failed to refresh Dropbox token: {refresh_response.text}")
|
||||
@@ -209,7 +210,8 @@ async def test_dropbox_token(request: Request):
|
||||
headers = {"Authorization": f"Bearer {access_token}"}
|
||||
response = requests.post(
|
||||
"https://api.dropboxapi.com/2/users/get_current_account",
|
||||
headers=headers
|
||||
headers=headers,
|
||||
timeout=settings.http_request_timeout
|
||||
)
|
||||
|
||||
if response.status_code != 200:
|
||||
|
||||
@@ -53,7 +53,7 @@ async def exchange_google_drive_token(
|
||||
|
||||
# Make the token request
|
||||
logger.info("Sending POST request to Google for token exchange")
|
||||
response = requests.post(token_url, data=payload)
|
||||
response = requests.post(token_url, data=payload, timeout=settings.http_request_timeout)
|
||||
|
||||
# Check if the request was successful
|
||||
logger.info(f"Token exchange response status: {response.status_code}")
|
||||
|
||||
+3
-3
@@ -55,7 +55,7 @@ async def exchange_onedrive_token(
|
||||
|
||||
# Make the token request
|
||||
logger.info("Sending POST request to Microsoft for token exchange")
|
||||
response = requests.post(token_url, data=payload)
|
||||
response = requests.post(token_url, data=payload, timeout=settings.http_request_timeout)
|
||||
|
||||
# Check if the request was successful
|
||||
logger.info(f"Token exchange response status: {response.status_code}")
|
||||
@@ -139,7 +139,7 @@ async def test_onedrive_token(request: Request):
|
||||
"scope": "offline_access Files.ReadWrite"
|
||||
}
|
||||
|
||||
response = requests.post(token_url, data=refresh_data)
|
||||
response = requests.post(token_url, data=refresh_data, timeout=settings.http_request_timeout)
|
||||
|
||||
if response.status_code != 200:
|
||||
logger.error(f"Failed to refresh OneDrive token: {response.text}")
|
||||
@@ -193,7 +193,7 @@ async def test_onedrive_token(request: Request):
|
||||
user_info_url = "https://graph.microsoft.com/v1.0/me"
|
||||
headers = {"Authorization": f"Bearer {access_token}"}
|
||||
|
||||
user_response = requests.get(user_info_url, headers=headers)
|
||||
user_response = requests.get(user_info_url, headers=headers, timeout=settings.http_request_timeout)
|
||||
|
||||
if user_response.status_code != 200:
|
||||
logger.error(f"OneDrive token test failed: {user_response.status_code} {user_response.text}")
|
||||
|
||||
+2
-1
@@ -23,7 +23,8 @@ async def whoami_handler(request: Request):
|
||||
raise HTTPException(status_code=400, detail="User has no email in session")
|
||||
|
||||
# Generate Gravatar URL from email
|
||||
email_hash = md5(email.strip().lower().encode()).hexdigest()
|
||||
# MD5 is used here for Gravatar's URL generation (not for security), so usedforsecurity=False
|
||||
email_hash = md5(email.strip().lower().encode(), usedforsecurity=False).hexdigest()
|
||||
gravatar_url = f"https://www.gravatar.com/avatar/{email_hash}?d=identicon"
|
||||
|
||||
# Add the gravatar URL to the user object instead of creating a new response
|
||||
|
||||
+2
-1
@@ -62,7 +62,8 @@ def require_login(func):
|
||||
def get_gravatar_url(email):
|
||||
"""Generate a Gravatar URL for the given email"""
|
||||
email = email.lower().strip()
|
||||
email_hash = hashlib.md5(email.encode('utf-8')).hexdigest()
|
||||
# MD5 is used here for Gravatar's URL generation (not for security), so usedforsecurity=False
|
||||
email_hash = hashlib.md5(email.encode('utf-8'), usedforsecurity=False).hexdigest()
|
||||
return f"https://www.gravatar.com/avatar/{email_hash}?d=identicon"
|
||||
|
||||
|
||||
|
||||
@@ -104,6 +104,9 @@ class Settings(BaseSettings):
|
||||
sftp_folder: Optional[str] = None
|
||||
sftp_private_key: Optional[str] = None
|
||||
sftp_private_key_passphrase: Optional[str] = None
|
||||
# Security: Disable host key verification only in development/testing environments
|
||||
# In production, set to True and configure known_hosts file
|
||||
sftp_disable_host_key_verification: bool = True # Default allows connection without known_hosts
|
||||
|
||||
# Email settings
|
||||
email_host: Optional[str] = None
|
||||
@@ -134,6 +137,9 @@ class Settings(BaseSettings):
|
||||
uptime_kuma_url: Optional[str] = None
|
||||
uptime_kuma_ping_interval: int = 5 # Default ping interval in minutes
|
||||
|
||||
# HTTP request settings
|
||||
http_request_timeout: int = 30 # Default timeout for HTTP requests in seconds
|
||||
|
||||
# Feature flags
|
||||
allow_file_delete: bool = True # Default to allowing file deletion from database
|
||||
|
||||
|
||||
@@ -189,7 +189,7 @@ def convert_to_pdf(self, file_path, original_filename=None):
|
||||
log_task_progress(task_id, "call_gotenberg", "in_progress", "Calling Gotenberg API")
|
||||
|
||||
# Send the conversion request to Gotenberg
|
||||
response = requests.post(endpoint, files=files, data=form_data)
|
||||
response = requests.post(endpoint, files=files, data=form_data, timeout=settings.http_request_timeout)
|
||||
|
||||
if response.status_code == 200:
|
||||
# Save the converted PDF
|
||||
|
||||
@@ -49,8 +49,8 @@ def get_dropbox_access_token():
|
||||
"client_id": settings.dropbox_app_key,
|
||||
"client_secret": settings.dropbox_app_secret,
|
||||
}
|
||||
|
||||
response = requests.post(token_url, headers=headers, data=data)
|
||||
|
||||
|
||||
response = requests.post(token_url, headers=headers, data=data, timeout=settings.http_request_timeout)
|
||||
|
||||
if response.status_code == 200:
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
import os
|
||||
import ftplib
|
||||
# Security Warning: FTP is an insecure protocol. FTPS (FTP_TLS) is strongly recommended.
|
||||
# This module attempts to use FTPS by default and falls back to plaintext FTP only if configured.
|
||||
import ftplib # nosec B402 - FTP usage is intentional for legacy server support
|
||||
from app.config import settings
|
||||
from app.tasks.retry_config import BaseTaskWithRetry
|
||||
from app.celery_app import celery
|
||||
@@ -15,6 +17,10 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
|
||||
"""
|
||||
Uploads a file to an FTP server in the configured folder.
|
||||
|
||||
Security Note: This function prefers FTPS (FTP with TLS) for secure connections.
|
||||
Plaintext FTP is only used if FTPS fails and ftp_allow_plaintext=True (default).
|
||||
For security-critical environments, set ftp_allow_plaintext=False and ftp_use_tls=True.
|
||||
|
||||
Args:
|
||||
file_path: Path to the file to upload
|
||||
file_id: Optional file ID to associate with logs
|
||||
@@ -71,8 +77,8 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
|
||||
raise Exception(error_msg)
|
||||
else:
|
||||
logger.warning(f"FTPS connection failed, falling back to regular FTP: {str(e)}")
|
||||
# Fall back to regular FTP
|
||||
ftp = ftplib.FTP()
|
||||
# Fall back to regular FTP - only if explicitly allowed by configuration
|
||||
ftp = ftplib.FTP() # nosec B321 - Fallback to FTP intentional when configured
|
||||
ftp.connect(
|
||||
host=settings.ftp_host,
|
||||
port=settings.ftp_port or 21
|
||||
@@ -91,7 +97,8 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
|
||||
raise Exception(error_msg)
|
||||
|
||||
# Directly use regular FTP if TLS is explicitly disabled
|
||||
ftp = ftplib.FTP()
|
||||
logger.warning("Using plaintext FTP - connection is NOT encrypted!")
|
||||
ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when explicitly configured
|
||||
ftp.connect(
|
||||
host=settings.ftp_host,
|
||||
port=settings.ftp_port or 21
|
||||
|
||||
@@ -134,7 +134,7 @@ def create_upload_session(filename, folder_path, access_token):
|
||||
|
||||
logger.info(f"Creating upload session for {filename} at path {folder_path}")
|
||||
|
||||
response = requests.post(url, headers=headers, json=request_body)
|
||||
response = requests.post(url, headers=headers, json=request_body, timeout=settings.http_request_timeout)
|
||||
|
||||
if response.status_code == 200:
|
||||
upload_url = response.json().get("uploadUrl")
|
||||
@@ -190,7 +190,8 @@ def upload_large_file(file_path, upload_url):
|
||||
response = requests.put(
|
||||
upload_url,
|
||||
headers=headers,
|
||||
data=chunk
|
||||
data=chunk,
|
||||
timeout=settings.http_request_timeout
|
||||
)
|
||||
|
||||
# Check if successful
|
||||
|
||||
@@ -49,7 +49,7 @@ def poll_task_for_document_id(task_id: str) -> int:
|
||||
|
||||
while attempts < POLL_MAX_ATTEMPTS:
|
||||
try:
|
||||
resp = requests.get(url, headers=_get_headers(), params={"task_id": task_id})
|
||||
resp = requests.get(url, headers=_get_headers(), params={"task_id": task_id}, timeout=settings.http_request_timeout)
|
||||
resp.raise_for_status()
|
||||
tasks_data = resp.json()
|
||||
except requests.exceptions.RequestException as exc:
|
||||
@@ -125,7 +125,7 @@ def upload_to_paperless(self, file_path: str, file_id: int = None):
|
||||
|
||||
try:
|
||||
logger.debug("Posting document to Paperless: file=%s", filename)
|
||||
resp = requests.post(post_url, headers=_get_headers(), files=files, data=data)
|
||||
resp = requests.post(post_url, headers=_get_headers(), files=files, data=data, timeout=settings.http_request_timeout)
|
||||
resp.raise_for_status()
|
||||
except requests.exceptions.RequestException as exc:
|
||||
error_msg = f"Failed to upload to Paperless: {exc}"
|
||||
|
||||
@@ -44,7 +44,20 @@ def upload_to_sftp(self, file_path: str, file_id: int = None):
|
||||
|
||||
# SSH client for SFTP connection
|
||||
ssh = paramiko.SSHClient()
|
||||
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
|
||||
|
||||
# Security: Host key verification
|
||||
# WARNING: AutoAddPolicy automatically trusts unknown host keys (vulnerable to MITM attacks)
|
||||
# For production, use RejectPolicy and configure known_hosts, or WarningPolicy at minimum
|
||||
if getattr(settings, 'sftp_disable_host_key_verification', True):
|
||||
logger.warning(
|
||||
"SFTP host key verification is DISABLED - connections are vulnerable to MITM attacks. "
|
||||
"For production, set SFTP_DISABLE_HOST_KEY_VERIFICATION=false and configure known_hosts."
|
||||
)
|
||||
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) # nosec B507 - Configurable, warns user
|
||||
else:
|
||||
# Use system known_hosts for host key verification (more secure)
|
||||
ssh.load_system_host_keys()
|
||||
ssh.set_missing_host_key_policy(paramiko.RejectPolicy())
|
||||
|
||||
try:
|
||||
# Setup connection parameters
|
||||
|
||||
@@ -64,7 +64,8 @@ def upload_to_webdav(self, file_path: str, file_id: int = None):
|
||||
webdav_url,
|
||||
auth=(settings.webdav_username, settings.webdav_password),
|
||||
data=file_data,
|
||||
verify=settings.webdav_verify_ssl if hasattr(settings, "webdav_verify_ssl") else True
|
||||
verify=settings.webdav_verify_ssl if hasattr(settings, "webdav_verify_ssl") else True,
|
||||
timeout=settings.http_request_timeout
|
||||
)
|
||||
|
||||
# Check if upload was successful
|
||||
|
||||
Reference in New Issue
Block a user