yihong1120 / Construction-Hazard-Detection

Enhances construction site safety using YOLOv8 for object detection, identifying hazards like workers without helmets or safety vests, and proximity to machinery or vehicles. HDBSCAN clusters safety cone coordinates to create monitored zones. Post-processing algorithms improve detection accuracy.
https://wiki.mutable.ai/yihong1120/Construction-Hazard-Detection
GNU Affero General Public License v3.0
189 stars 18 forks source link

Fix code scanning alert - Information exposure through an exception #14

Closed yihong1120 closed 1 month ago

yihong1120 commented 1 month ago

Tracking issue for:

codeautopilot[bot] commented 1 month ago

Potential solution

The plan to solve the bug involves adding proper exception handling and logging mechanisms to ensure that sensitive information is not exposed through exceptions. This will be done by updating the code in the relevant files to catch exceptions, log them securely, and return generic error messages to the client.

What is causing this bug?

The bug is caused by the lack of proper exception handling in the code, which can lead to sensitive information being exposed through exception messages. Specifically, the model_downloader.py file includes detailed exception messages in the JSON responses, which can reveal sensitive information about the server or the request.

Code

Here are the implementation details and code snippets to address the issue:

examples/YOLOv8_server_api/security.py

Add exception handling and logging to the update_secret_key function to ensure that any potential issues are securely managed.

import logging
import secrets
from flask import Flask

def update_secret_key(app: Flask) -> None:
    """
    Updates the JWT secret key in the application configuration.

    This function generates a new, secure JWT secret key and updates the
    Flask application's config with this token. It helps protect against
    security breaches by regularly rotating the secret key.

    Args:
        app (Flask): The Flask app instance to update the JWT secret key.
    """
    try:
        # Securely generate a new, random JWT secret key using the secrets library
        app.config['JWT_SECRET_KEY'] = secrets.token_urlsafe(16)
    except Exception as e:
        logging.error("Failed to update JWT secret key: %s", str(e))
        # Optionally, re-raise the exception if it should be handled further up the stack
        raise

examples/YOLOv8_server_api/auth.py

Ensure that detailed error information is logged securely for debugging purposes, but not exposed to the user.

import logging
from flask import Blueprint, request, jsonify, Response
from flask_jwt_extended import create_access_token

auth_blueprint = Blueprint('auth', __name__)
logger = logging.getLogger(__name__)

@auth_blueprint.route('/token', methods=['POST'])
def create_token() -> Response:
    try:
        username = request.json.get('username', None)
        password = request.json.get('password', None)

        user = user_cache.get(username)
        if not user:
            user = User.query.filter_by(username=username).first()
            if user:
                user_cache[username] = user

        if not user or not user.check_password(password):
            logger.warning(f'Failed login attempt for username: {username}')
            response = jsonify({'msg': 'Wrong user name or passcode.'})
            response.status_code = 401
            return response

        access_token = create_access_token(identity=username)
        return jsonify(access_token=access_token)
    except Exception as e:
        logger.error(f'Error during authentication: {str(e)}')
        response = jsonify({'msg': 'Internal server error.'})
        response.status_code = 500
        return response

examples/YOLOv8_server_api/app.py

Implement a global error handler and custom error pages to catch and handle exceptions gracefully without exposing sensitive information.

from flask import Flask, jsonify

app = Flask(__name__)

# Global error handler
@app.errorhandler(Exception)
def handle_exception(e):
    # Log the exception details (ensure sensitive information is not logged)
    app.logger.error(f"An error occurred: {str(e)}")

    # Return a generic error message
    response = {
        "message": "An internal error occurred. Please try again later."
    }
    return jsonify(response), 500

# Custom error handler for 404 Not Found
@app.errorhandler(404)
def not_found_error(error):
    response = {
        "message": "Resource not found."
    }
    return jsonify(response), 404

# Custom error handler for 500 Internal Server Error
@app.errorhandler(500)
def internal_error(error):
    response = {
        "message": "An internal error occurred. Please try again later."
    }
    return jsonify(response), 500

examples/YOLOv8_server_api/model_downloader.py

Remove detailed error information from the JSON response and log the error details internally.

from __future__ import annotations

import datetime
from pathlib import Path

import requests
from flask import Blueprint, jsonify, send_from_directory, current_app as app
from flask_limiter import Limiter
from flask_limiter.util import get_remote_address

models_blueprint = Blueprint('models', __name__)
limiter = Limiter(key_func=get_remote_address)

@models_blueprint.route('/models/<model_name>', methods=['GET'])
@limiter.limit('10 per minute')
def download_model(model_name):
    """
    Endpoint to download model files.

    Args:
        model_name (str): The name of the model file to download.

    Returns:
        Response: A Flask response object that downloads the model file.
    """
    # Define the directory where the model files are stored
    MODELS_DIRECTORY = 'models/pt/'
    # Define the external URL for model files
    MODEL_URL = f"http://changdar-server.mooo.com:28000/models/{model_name}"

    # Ensure the model name is valid
    allowed_models = ['best_yolov8l.pt', 'best_yolov8x.pt']
    if model_name not in allowed_models:
        return jsonify({'error': 'Model not found.'}), 404

    try:
        # Check last modified time via a HEAD request to the external server
        response = requests.head(MODEL_URL)
        if response.status_code == 200 and 'Last-Modified' in response.headers:
            server_last_modified = datetime.datetime.strptime(
                response.headers['Last-Modified'],
                '%a, %d %b %Y %H:%M:%S GMT',
            )
            local_file_path = f"{MODELS_DIRECTORY}{model_name}"
            # Check local file's last modified time
            local_last_modified = datetime.datetime.fromtimestamp(
                Path(local_file_path).stat().st_mtime,
            )
            if local_last_modified >= server_last_modified:
                return jsonify({'message': 'Local model is up-to-date.'}), 304

        # If not up-to-date, fetch the file and return it
        return send_from_directory(
            MODELS_DIRECTORY, model_name, as_attachment=True,
        )

    except FileNotFoundError:
        return jsonify({'error': 'Model not found.'}), 404
    except requests.RequestException:
        # Log the exception details internally
        app.logger.error('Failed to fetch model information', exc_info=True)
        return (
            jsonify(
                {
                    'error': 'Failed to fetch model information.',
                },
            ),
            500,
        )

How to replicate the bug

  1. Deploy the YOLOv8 server API application.
  2. Trigger an exception in any of the endpoints, such as by providing invalid input or causing a network error.
  3. Observe the error message returned in the JSON response. If the message contains detailed exception information, the bug is present.

By implementing the above changes, the application will handle exceptions securely, log necessary details internally, and return generic error messages to the client, thus preventing information exposure through exceptions.

Click here to create a Pull Request with the proposed solution

Files used for this task:

Changes on examples/YOLOv8_server_api/security.py # Analysis Report for `examples/YOLOv8_server_api/security.py` ## Overview The file `security.py` contains a function `update_secret_key` that updates the JWT secret key in the Flask application's configuration. This function uses the `secrets` library to generate a new, secure JWT secret key. ## Code Review ### Function: `update_secret_key` ```python def update_secret_key(app: Flask) -> None: """ Updates the JWT secret key in the application configuration. This function generates a new, secure JWT secret key and updates the Flask application's config with this token. It helps protect against security breaches by regularly rotating the secret key. Args: app (Flask): The Flask app instance to update the JWT secret key. """ # Securely generate a new, random JWT secret key using the secrets library app.config['JWT_SECRET_KEY'] = secrets.token_urlsafe(16) ``` ### Exception Handling - The function `update_secret_key` does not currently include any exception handling. - Given the nature of the function, potential exceptions could arise from: - Issues with the Flask app instance (e.g., if `app` is `None` or improperly configured). - Issues with the `secrets` library (though this is less likely as it is part of the Python standard library). ### Potential Information Exposure - If an exception were to occur, it could potentially expose sensitive information if not handled properly. - However, this function does not currently log or print any information, so the risk of information exposure through this function is minimal. ## Recommendations 1. **Add Exception Handling**: To ensure that sensitive information is not exposed, add exception handling to catch and log errors without exposing sensitive details. 2. **Logging**: Use a logging mechanism to log errors in a secure manner. ### Updated Code with Exception Handling ```python import logging def update_secret_key(app: Flask) -> None: """ Updates the JWT secret key in the application configuration. This function generates a new, secure JWT secret key and updates the Flask application's config with this token. It helps protect against security breaches by regularly rotating the secret key. Args: app (Flask): The Flask app instance to update the JWT secret key. """ try: # Securely generate a new, random JWT secret key using the secrets library app.config['JWT_SECRET_KEY'] = secrets.token_urlsafe(16) except Exception as e: logging.error("Failed to update JWT secret key: %s", str(e)) # Optionally, re-raise the exception if it should be handled further up the stack raise ``` ## Conclusion The file `security.py` does not currently expose sensitive information through exceptions, but adding exception handling and logging will further ensure that any potential issues are securely managed. The provided updated code includes these improvements.
Changes on examples/YOLOv8_server_api/auth.py # Analysis Report for `examples/YOLOv8_server_api/auth.py` ## Overview The file `auth.py` is responsible for handling user authentication in the YOLOv8 server API. It includes a route to create a JWT token after validating the user's credentials. ## Potential Issue The primary concern is to ensure that sensitive information is not exposed through exception messages. In this file, the main area to review is the exception handling in the `create_token` function. ## Code Review ### Function: `create_token` This function performs the following steps: 1. Extracts the username and password from the request JSON body. 2. Attempts to fetch the user details from a cache. 3. If the user is not in the cache, it queries the database. 4. Verifies the user's password. 5. Generates a JWT token if authentication is successful. 6. Returns the JWT token or an error message. ### Current Exception Handling - If the username or password is incorrect, the function returns a JSON response with the message `'Wrong user name or passcode.'` and a status code of 401. ### Potential Information Exposure - The current error message `'Wrong user name or passcode.'` is generic and does not expose sensitive information. This is good practice as it does not reveal whether the username or password was incorrect, which could be useful information for an attacker. ## Recommendations The current implementation of exception handling in this file appears to be secure and does not expose sensitive information. However, to further ensure robustness, consider the following: 1. **Logging**: Ensure that detailed error information is logged securely for debugging purposes, but not exposed to the user. 2. **Consistent Error Messages**: Maintain the use of generic error messages for all authentication-related errors. ### Example of Secure Logging (if not already implemented) ```python import logging logger = logging.getLogger(__name__) @auth_blueprint.route('/token', methods=['POST']) def create_token() -> Response: try: username = request.json.get('username', None) password = request.json.get('password', None) user = user_cache.get(username) if not user: user = User.query.filter_by(username=username).first() if user: user_cache[username] = user if not user or not user.check_password(password): logger.warning(f'Failed login attempt for username: {username}') response = jsonify({'msg': 'Wrong user name or passcode.'}) response.status_code = 401 return response access_token = create_access_token(identity=username) return jsonify(access_token=access_token) except Exception as e: logger.error(f'Error during authentication: {str(e)}') response = jsonify({'msg': 'Internal server error.'}) response.status_code = 500 return response ``` ## Conclusion The `auth.py` file currently handles exceptions in a secure manner by providing generic error messages. No sensitive information is exposed through the error messages. The addition of secure logging can further enhance the robustness of the error handling mechanism.
Changes on examples/YOLOv8_server_api/app.py ## Analysis Report for `examples/YOLOv8_server_api/app.py` ### Overview The `app.py` file is responsible for initializing and configuring the Flask application. It sets up various components such as JWT authentication, database connections, and background scheduling. The file also registers different blueprints for handling routes related to authentication, object detection, and model management. ### Key Components 1. **Flask Application Initialization**: The Flask app is initialized and configured with a randomly generated JWT secret key. 2. **JWT Manager**: JWTManager is initialized to handle JWT-based authentication. 3. **Database Initialization**: The database is initialized with the Flask app. 4. **Blueprint Registration**: Blueprints for authentication, detection, and models are registered. 5. **Background Scheduler**: A background scheduler is set up to update the JWT secret key every 30 days. 6. **Graceful Shutdown**: The scheduler is configured to shut down gracefully when the application exits. ### Exception Handling The file does not contain explicit exception handling mechanisms. Given the nature of the issue (information exposure through an exception), it is crucial to ensure that any exceptions that occur do not expose sensitive information. ### Potential Areas of Concern 1. **Flask Application Errors**: If the Flask application encounters an error, it might expose sensitive information in the error messages. 2. **Scheduler Errors**: Errors in the background scheduler could potentially expose sensitive information. 3. **Blueprint Registration**: Errors during the registration of blueprints might expose sensitive information. ### Recommendations 1. **Global Error Handler**: Implement a global error handler to catch and handle exceptions gracefully without exposing sensitive information. 2. **Custom Error Pages**: Configure custom error pages for common HTTP errors (e.g., 404, 500) to avoid exposing stack traces or sensitive details. 3. **Logging**: Ensure that sensitive information is not logged. Configure logging to capture necessary details without exposing sensitive data. ### Example Implementation Here is an example of how you can implement a global error handler and custom error pages: ```python from flask import jsonify # Global error handler @app.errorhandler(Exception) def handle_exception(e): # Log the exception details (ensure sensitive information is not logged) app.logger.error(f"An error occurred: {str(e)}") # Return a generic error message response = { "message": "An internal error occurred. Please try again later." } return jsonify(response), 500 # Custom error handler for 404 Not Found @app.errorhandler(404) def not_found_error(error): response = { "message": "Resource not found." } return jsonify(response), 404 # Custom error handler for 500 Internal Server Error @app.errorhandler(500) def internal_error(error): response = { "message": "An internal error occurred. Please try again later." } return jsonify(response), 500 ``` ### Conclusion The `app.py` file does not currently handle exceptions explicitly, which could lead to information exposure through error messages. Implementing a global error handler and custom error pages will help mitigate this risk by ensuring that sensitive information is not exposed in error messages.
Changes on examples/YOLOv8_server_api/model_downloader.py # Analysis Report for `examples/YOLOv8_server_api/model_downloader.py` ## Overview The file `model_downloader.py` contains a Flask blueprint for handling model file downloads. It includes an endpoint `/models/` that allows users to download specific model files. The endpoint also checks if the local model file is up-to-date with the server version before serving it. ## Potential Issue The primary concern is the exposure of sensitive information through exception handling. Specifically, the error messages returned in the JSON responses may contain details that could be exploited by an attacker. ## Detailed Analysis ### Current Exception Handling 1. **Model Not Found** ```python if model_name not in allowed_models: return jsonify({'error': 'Model not found.'}), 404 ``` - **Risk**: Low. This message does not expose sensitive information. 2. **File Not Found** ```python except FileNotFoundError: return jsonify({'error': 'Model not found.'}), 404 ``` - **Risk**: Low. This message is generic and does not expose sensitive information. 3. **Request Exception** ```python except requests.RequestException as e: return ( jsonify( { 'error': 'Failed to fetch model information.', 'details': str(e), }, ), 500, ) ``` - **Risk**: High. The `details` field includes the exception message, which may contain sensitive information about the server or the request. ## Recommendations 1. **Remove Detailed Error Information**: Avoid exposing the exception details in the response. Instead, log the error details internally and return a generic error message to the client. ### Updated Code ```python from __future__ import annotations import datetime from pathlib import Path import requests from flask import Blueprint from flask import jsonify from flask import send_from_directory from flask_limiter import Limiter from flask_limiter.util import get_remote_address models_blueprint = Blueprint('models', __name__) limiter = Limiter(key_func=get_remote_address) @models_blueprint.route('/models/', methods=['GET']) @limiter.limit('10 per minute') def download_model(model_name): """ Endpoint to download model files. Args: model_name (str): The name of the model file to download. Returns: Response: A Flask response object that downloads the model file. """ # Define the directory where the model files are stored MODELS_DIRECTORY = 'models/pt/' # Define the external URL for model files MODEL_URL = f"http://changdar-server.mooo.com:28000/models/{model_name}" # Ensure the model name is valid allowed_models = ['best_yolov8l.pt', 'best_yolov8x.pt'] if model_name not in allowed_models: return jsonify({'error': 'Model not found.'}), 404 try: # Check last modified time via a HEAD request to the external server response = requests.head(MODEL_URL) if response.status_code == 200 and 'Last-Modified' in response.headers: server_last_modified = datetime.datetime.strptime( response.headers['Last-Modified'], '%a, %d %b %Y %H:%M:%S GMT', ) local_file_path = f"{MODELS_DIRECTORY}{model_name}" # Check local file's last modified time local_last_modified = datetime.datetime.fromtimestamp( Path(local_file_path).stat().st_mtime, ) if local_last_modified >= server_last_modified: return jsonify({'message': 'Local model is up-to-date.'}), 304 # If not up-to-date, fetch the file and return it return send_from_directory( MODELS_DIRECTORY, model_name, as_attachment=True, ) except FileNotFoundError: return jsonify({'error': 'Model not found.'}), 404 except requests.RequestException: # Log the exception details internally app.logger.error('Failed to fetch model information', exc_info=True) return ( jsonify( { 'error': 'Failed to fetch model information.', }, ), 500, ) ``` ### Summary - Removed the detailed exception message from the JSON response. - Added internal logging for the exception details to ensure that the information is available for debugging without exposing it to the client. This update ensures that sensitive information is not exposed through exception handling while maintaining the ability to debug issues internally.