security: implement HASH-001 and ENUM-001 remediations
HASH-001: Add threading lock to content hash deduplication - Prevents race condition between SELECT and UPDATE - Ensures accurate dedup counting under concurrent load ENUM-001: Add rate limiting to paste lookups - Separate rate limiter for GET/HEAD on paste endpoints - Default 60 requests/minute per IP (configurable) - Prevents brute-force paste ID enumeration attacks
This commit is contained in:
@@ -250,9 +250,9 @@ Testing uses specialized Claude subagents for different security domains, with f
|
|||||||
|
|
||||||
### Medium-term (Medium)
|
### Medium-term (Medium)
|
||||||
|
|
||||||
- [ ] **HASH-001**: Add locking to content hash deduplication
|
- [x] **HASH-001**: Add locking to content hash deduplication
|
||||||
- [x] **FLOOD-001**: Add memory limit to anti-flood request list
|
- [x] **FLOOD-001**: Add memory limit to anti-flood request list
|
||||||
- [ ] **ENUM-001**: Add rate limiting to paste metadata endpoints
|
- [x] **ENUM-001**: Add rate limiting to paste metadata endpoints
|
||||||
- [x] **CLI-002**: Verify SSL certificate hostname matching
|
- [x] **CLI-002**: Verify SSL certificate hostname matching
|
||||||
- [x] **CLI-003**: Add config file permission validation on startup
|
- [x] **CLI-003**: Add config file permission validation on startup
|
||||||
- [x] **AUDIT-001**: Add query result limits to prevent enumeration
|
- [x] **AUDIT-001**: Add query result limits to prevent enumeration
|
||||||
|
|||||||
@@ -273,6 +273,50 @@ def reset_rate_limits() -> None:
|
|||||||
_rate_limit_requests.clear()
|
_rate_limit_requests.clear()
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────────────────────
|
||||||
|
# ENUM-001: Lookup Rate Limiting (prevents paste ID enumeration)
|
||||||
|
# ─────────────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
_lookup_rate_limit_lock = threading.Lock()
|
||||||
|
_lookup_rate_limit_requests: dict[str, list[float]] = defaultdict(list)
|
||||||
|
|
||||||
|
|
||||||
|
def check_lookup_rate_limit(client_ip: str) -> tuple[bool, int]:
|
||||||
|
"""Check if lookup request is within rate limit.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
client_ip: Client IP address
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
Tuple of (allowed, retry_after_seconds)
|
||||||
|
"""
|
||||||
|
if not current_app.config.get("LOOKUP_RATE_LIMIT_ENABLED", True):
|
||||||
|
return True, 0
|
||||||
|
|
||||||
|
window = current_app.config.get("LOOKUP_RATE_LIMIT_WINDOW", 60)
|
||||||
|
max_requests = current_app.config.get("LOOKUP_RATE_LIMIT_MAX", 60)
|
||||||
|
|
||||||
|
now = time.time()
|
||||||
|
cutoff = now - window
|
||||||
|
|
||||||
|
with _lookup_rate_limit_lock:
|
||||||
|
requests = _lookup_rate_limit_requests[client_ip]
|
||||||
|
requests[:] = [t for t in requests if t > cutoff]
|
||||||
|
|
||||||
|
if len(requests) >= max_requests:
|
||||||
|
retry_after = int(requests[0] + window - now) + 1
|
||||||
|
return False, max(1, retry_after)
|
||||||
|
|
||||||
|
requests.append(now)
|
||||||
|
return True, 0
|
||||||
|
|
||||||
|
|
||||||
|
def reset_lookup_rate_limits() -> None:
|
||||||
|
"""Clear lookup rate limit state. For testing only."""
|
||||||
|
with _lookup_rate_limit_lock:
|
||||||
|
_lookup_rate_limit_requests.clear()
|
||||||
|
|
||||||
|
|
||||||
def add_rate_limit_headers(
|
def add_rate_limit_headers(
|
||||||
response: Response, remaining: int, limit: int, reset_timestamp: int
|
response: Response, remaining: int, limit: int, reset_timestamp: int
|
||||||
) -> Response:
|
) -> Response:
|
||||||
@@ -415,6 +459,18 @@ def validate_paste_id(paste_id: str) -> Response | None:
|
|||||||
|
|
||||||
def fetch_paste(paste_id: str, check_password: bool = True) -> Response | None:
|
def fetch_paste(paste_id: str, check_password: bool = True) -> Response | None:
|
||||||
"""Fetch paste and store in g.paste. Returns error response or None if OK."""
|
"""Fetch paste and store in g.paste. Returns error response or None if OK."""
|
||||||
|
# ENUM-001: Rate limit lookups to prevent enumeration attacks
|
||||||
|
client_ip = get_client_ip()
|
||||||
|
allowed, retry_after = check_lookup_rate_limit(client_ip)
|
||||||
|
if not allowed:
|
||||||
|
response = error_response(
|
||||||
|
f"Lookup rate limit exceeded. Retry after {retry_after} seconds.",
|
||||||
|
429,
|
||||||
|
retry_after=retry_after,
|
||||||
|
)
|
||||||
|
response.headers["Retry-After"] = str(retry_after)
|
||||||
|
return response
|
||||||
|
|
||||||
db = get_db()
|
db = get_db()
|
||||||
now = int(time.time())
|
now = int(time.time())
|
||||||
|
|
||||||
|
|||||||
@@ -108,6 +108,16 @@ class Config:
|
|||||||
os.environ.get("FLASKPASTE_RATE_CLEANUP_THRESHOLD", "0.8")
|
os.environ.get("FLASKPASTE_RATE_CLEANUP_THRESHOLD", "0.8")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# ENUM-001: Rate limiting for paste lookups (prevents enumeration attacks)
|
||||||
|
# Separate from creation limits - allows more reads but prevents brute-force
|
||||||
|
LOOKUP_RATE_LIMIT_ENABLED = os.environ.get("FLASKPASTE_LOOKUP_RATE_LIMIT", "1").lower() in (
|
||||||
|
"1",
|
||||||
|
"true",
|
||||||
|
"yes",
|
||||||
|
)
|
||||||
|
LOOKUP_RATE_LIMIT_WINDOW = int(os.environ.get("FLASKPASTE_LOOKUP_RATE_WINDOW", "60"))
|
||||||
|
LOOKUP_RATE_LIMIT_MAX = int(os.environ.get("FLASKPASTE_LOOKUP_RATE_MAX", "60"))
|
||||||
|
|
||||||
# Audit Logging
|
# Audit Logging
|
||||||
# Track security-relevant events (paste creation, deletion, rate limits, etc.)
|
# Track security-relevant events (paste creation, deletion, rate limits, etc.)
|
||||||
AUDIT_ENABLED = os.environ.get("FLASKPASTE_AUDIT", "1").lower() in ("1", "true", "yes")
|
AUDIT_ENABLED = os.environ.get("FLASKPASTE_AUDIT", "1").lower() in ("1", "true", "yes")
|
||||||
@@ -154,6 +164,11 @@ class TestingConfig(Config):
|
|||||||
RATE_LIMIT_WINDOW = 1
|
RATE_LIMIT_WINDOW = 1
|
||||||
RATE_LIMIT_MAX = 100
|
RATE_LIMIT_MAX = 100
|
||||||
|
|
||||||
|
# Relaxed lookup rate limiting for tests (ENUM-001)
|
||||||
|
LOOKUP_RATE_LIMIT_ENABLED = True
|
||||||
|
LOOKUP_RATE_LIMIT_WINDOW = 1
|
||||||
|
LOOKUP_RATE_LIMIT_MAX = 1000
|
||||||
|
|
||||||
# PKI testing configuration
|
# PKI testing configuration
|
||||||
PKI_ENABLED = True
|
PKI_ENABLED = True
|
||||||
PKI_CA_PASSWORD = "test-ca-password"
|
PKI_CA_PASSWORD = "test-ca-password"
|
||||||
|
|||||||
@@ -5,12 +5,16 @@ from __future__ import annotations
|
|||||||
import hashlib
|
import hashlib
|
||||||
import secrets
|
import secrets
|
||||||
import sqlite3
|
import sqlite3
|
||||||
|
import threading
|
||||||
import time
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import TYPE_CHECKING, Any
|
from typing import TYPE_CHECKING, Any
|
||||||
|
|
||||||
from flask import current_app, g
|
from flask import current_app, g
|
||||||
|
|
||||||
|
# HASH-001: Lock for content hash deduplication to prevent race conditions
|
||||||
|
_content_hash_lock = threading.Lock()
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from flask import Flask
|
from flask import Flask
|
||||||
|
|
||||||
@@ -264,44 +268,47 @@ def check_content_hash(content_hash: str) -> tuple[bool, int]:
|
|||||||
|
|
||||||
db = get_db()
|
db = get_db()
|
||||||
|
|
||||||
# Check existing hash record
|
# HASH-001: Lock to prevent race condition between SELECT and UPDATE
|
||||||
row = db.execute(
|
with _content_hash_lock:
|
||||||
"SELECT count, last_seen FROM content_hashes WHERE hash = ?", (content_hash,)
|
# Check existing hash record
|
||||||
).fetchone()
|
row = db.execute(
|
||||||
|
"SELECT count, last_seen FROM content_hashes WHERE hash = ?", (content_hash,)
|
||||||
|
).fetchone()
|
||||||
|
|
||||||
if row is None:
|
if row is None:
|
||||||
# First time seeing this content
|
# First time seeing this content
|
||||||
|
db.execute(
|
||||||
|
"INSERT INTO content_hashes (hash, first_seen, last_seen, count) "
|
||||||
|
"VALUES (?, ?, ?, 1)",
|
||||||
|
(content_hash, now, now),
|
||||||
|
)
|
||||||
|
db.commit()
|
||||||
|
return True, 1
|
||||||
|
|
||||||
|
if row["last_seen"] < cutoff:
|
||||||
|
# Outside window, reset counter
|
||||||
|
db.execute(
|
||||||
|
"UPDATE content_hashes SET first_seen = ?, last_seen = ?, count = 1 WHERE hash = ?",
|
||||||
|
(now, now, content_hash),
|
||||||
|
)
|
||||||
|
db.commit()
|
||||||
|
return True, 1
|
||||||
|
|
||||||
|
# Within window, check threshold
|
||||||
|
current_count = row["count"] + 1
|
||||||
|
|
||||||
|
if current_count > max_count:
|
||||||
|
# Exceeded threshold, don't increment (prevent counter overflow)
|
||||||
|
return False, row["count"]
|
||||||
|
|
||||||
|
# Update counter
|
||||||
db.execute(
|
db.execute(
|
||||||
"INSERT INTO content_hashes (hash, first_seen, last_seen, count) VALUES (?, ?, ?, 1)",
|
"UPDATE content_hashes SET last_seen = ?, count = ? WHERE hash = ?",
|
||||||
(content_hash, now, now),
|
(now, current_count, content_hash),
|
||||||
)
|
)
|
||||||
db.commit()
|
db.commit()
|
||||||
return True, 1
|
|
||||||
|
|
||||||
if row["last_seen"] < cutoff:
|
return True, current_count
|
||||||
# Outside window, reset counter
|
|
||||||
db.execute(
|
|
||||||
"UPDATE content_hashes SET first_seen = ?, last_seen = ?, count = 1 WHERE hash = ?",
|
|
||||||
(now, now, content_hash),
|
|
||||||
)
|
|
||||||
db.commit()
|
|
||||||
return True, 1
|
|
||||||
|
|
||||||
# Within window, check threshold
|
|
||||||
current_count = row["count"] + 1
|
|
||||||
|
|
||||||
if current_count > max_count:
|
|
||||||
# Exceeded threshold, don't increment (prevent counter overflow)
|
|
||||||
return False, row["count"]
|
|
||||||
|
|
||||||
# Update counter
|
|
||||||
db.execute(
|
|
||||||
"UPDATE content_hashes SET last_seen = ?, count = ? WHERE hash = ?",
|
|
||||||
(now, current_count, content_hash),
|
|
||||||
)
|
|
||||||
db.commit()
|
|
||||||
|
|
||||||
return True, current_count
|
|
||||||
|
|
||||||
|
|
||||||
def init_app(app: Flask) -> None:
|
def init_app(app: Flask) -> None:
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ import pytest
|
|||||||
|
|
||||||
import app.database as db_module
|
import app.database as db_module
|
||||||
from app import create_app
|
from app import create_app
|
||||||
from app.api.routes import reset_rate_limits
|
from app.api.routes import reset_lookup_rate_limits, reset_rate_limits
|
||||||
|
|
||||||
|
|
||||||
def _clear_database():
|
def _clear_database():
|
||||||
@@ -22,6 +22,7 @@ def app():
|
|||||||
"""Create application for testing."""
|
"""Create application for testing."""
|
||||||
# Reset global state for test isolation
|
# Reset global state for test isolation
|
||||||
reset_rate_limits()
|
reset_rate_limits()
|
||||||
|
reset_lookup_rate_limits()
|
||||||
_clear_database()
|
_clear_database()
|
||||||
|
|
||||||
test_app = create_app("testing")
|
test_app = create_app("testing")
|
||||||
@@ -33,6 +34,7 @@ def app():
|
|||||||
|
|
||||||
# Cleanup after test
|
# Cleanup after test
|
||||||
reset_rate_limits()
|
reset_rate_limits()
|
||||||
|
reset_lookup_rate_limits()
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
|
|||||||
Reference in New Issue
Block a user