Dev

[skye-cyber]

sync main and dev

The vulnerability arises because untrusted user input (via form data or other means) can supply file paths in cli_args, some of which may not have been validated or sanitized. While uploaded filenames are vetted and restricted to a specific temp directory, form data could also provide paths that are concatenated into cli_args, especially for certain tools or extended usage.

General fix:
For every file path extracted from cli_args for local access (such as with os.path.isfile in execute_cli_command), ensure the path is:

• Normalized,
• Confirmed to reside within the designated temp_dir (or other root directory).

Best fix:
In execute_cli_command, when iterating over file paths (line 224), normalize each path and check that it is strictly within temp_dir before accessing it. If a file path does not meet this criterion, skip it or raise an appropriate error. Also, generate output files only in temp_dir, never next to the original if it could be elsewhere.

Files/regions to change:
Only modify fweb/core/views.py, specifically in the method execute_cli_command, lines around 224-225.
Add path normalization and containment checks, ensuring that file access is only within temp_dir.

Needed imports/methods:
You may need to use os.path.normpath, os.path.abspath, and retain os as imported already.

---

The best way to fix this problem is to validate the output file path before writing to it, ensuring that it resides within the temporary processing directory (temp_dir). This can be done by normalizing the path (with os.path.normpath) and checking that it starts with the absolute path of temp_dir plus a path separator (to prevent prefix attacks). Specifically, before opening output_file for writing, verify that it is contained within temp_dir. If the check fails, skip file writing and optionally log a warning or error. This fix can be made within the execute_cli_command function, right before line 228.

To fix the problem, all instances where exception messages are returned directly to the user must be replaced with a generic message, while logging the actual exception (and stack trace) on the server for debugging purposes. Specifically:

• In process_with_cli, replace {"success": False, "error": str(e)} with a generic message (e.g., "error": "CLI processing failed."), while logging the full exception (optionally with exc_info=True for full stack trace).
• In process_tool, any usage of values in result["error"] (which may contain an exception message) should not be exposed directly to the user; replace it with a generic message (e.g., "An internal error has occurred.").
• Always ensure logging retains as much information as needed to debug.

No change is required for successful operations (result["message"] et al) since these come from normal, non-exceptional flow.
No new imports are needed, as logging is already imported.
Edits should be made in fweb/core/views.py in both process_tool and process_with_cli as described.

---

To resolve this issue, generic error messages rather than raw exception messages should be sent to users. All exception messages and stack traces should be logged server-side for debugging/auditing purposes using the available logger (logger.error(...)), but the API should respond only with a generic error message. This applies to both process_tool and its helper process_with_cli, which currently propagate exception details in the response.

We should:

• Replace {"success": False, "error": result["error"]} (and similar) with a generic error message to the user, such as {"success": False, "error": "An internal error has occurred."}
• Log detailed errors and exceptions server-side.
• If there is legitimate user-facing error information (e.g., validation errors), separate those from internal errors.

Edits are needed at:

• line 119 (and possibly 143 if a web-facing message ever directly propagates from process_with_cli to the client)

No new imports are needed because logging is already set up.

---

To fix this problem, we need to replace the user-facing error message in the exception handler (in process_tool) to return a generic message, rather than exposing the exception string. The server-side logging should retain the detailed exception for debugging. Only the JsonResponse on line 124 needs to change: remove str(e) from the response body and substitute a generic error message such as "An internal error occurred.". No other functional logic needs changing. No new imports or dependencies are needed because Django logging and JsonResponse are already present.

---

To fix this issue, we should ensure that the value looked up in toolConfigs is both an own property (not inherited from the prototype) and actually a function before calling it. This is best done by replacing if (toolConfigs[toolId]) with an explicit hasOwnProperty check and verifying the type of the property. The fix should occur in the initializeTool method, in file fweb/core/static/js/ToolsHandler.js, specifically lines 117-121. No external imports are needed since hasOwnProperty and typeof are both built-in JavaScript operations.

To safely perform a dynamic method call based on user input, we should strictly validate that the requested method name is both an own property of the methods dictionary (not inherited from a prototype) and that the mapped value is a function. The most direct way to fix the issue in initializeTool(toolId) is to replace the current if (setupFunctions[toolId]) check with:

• First, check that toolId is an own property of setupFunctions using Object.prototype.hasOwnProperty.call(setupFunctions, toolId)
• Then, check that setupFunctions[toolId] is of type "function".

Only if both conditions are true should the code invoke setupFunctions[toolId](). Otherwise, do nothing, or optionally show an error if needed.

You only need to change the relevant check and invocation in the initializeTool(toolId) function. No imports or extra definitions are needed; just use plain JS.

To fix the problem, we must ensure that the user-controlled toolId is only allowed to select valid initialization functions and that the dynamic lookup cannot resolve to prototype methods or unintentional targets. The best way is to check that toolId is an own property of initializationFunctions (not inherited), and that the resolved property is a function before calling it. Ideally, we would use a Map, but since the code uses a plain object, a strict check via hasOwnProperty and typeof is sufficient. Specifically, in initializeTool (lines 342-344), replace:

if (initializationFunctions[toolId]) {
  initializationFunctions[toolId]();
}

with:

if (
  Object.prototype.hasOwnProperty.call(initializationFunctions, toolId) &&
  typeof initializationFunctions[toolId] === "function"
) {
  initializationFunctions[toolId]();
}

Apply the same logic to the earlier setupFunctions dispatch (lines 150-152). No new imports are needed as this uses built-in JS features.

To fix this vulnerability, ensure that all variables that are interpolated into HTML via innerHTML are properly escaped, especially user-controllable values. In this case, escape file.name and (file.size / 1024 / 1024).toFixed(2) (even though the latter is always a number, safe input handling suggests escaping anyway for completeness). The best approach is to create a helper function (e.g., escapeHTML) that converts special HTML characters (&, <, >, ", ', `) into their corresponding HTML entities. Use this function to escape all interpolated user data (${file.name}, and for completeness, also ${dropZoneId} in an onclick, though it's less directly risky here because the index is always a number and the dropZoneId comes from your own application context).

Make these changes only within the provided snippet of fweb/core/static/js/FileHandler.js:

• Add an escapeHTML(str) utility function to the class.
• Change the construction of the HTML template in createFileItem to escape file.name before inclusion.

---

How to fix the problem in general terms:
Any data taken from untrusted sources which is to be output into innerHTML (or equivalent sinks in the DOM) must be escaped, or, better, should be output using safe DOM methods. Instead of using .innerHTML and template strings containing dynamic user data, use text nodes to insert data (textContent or createTextNode). This prevents meta-characters from being interpreted as HTML.

Detailed fix for this case:

• In function updateFileList, where the DOM for each file is being built (lines 39-45), switch to creating the structure with DOM methods.
• Specifically, create a <span> element for the file name, set its textContent (not innerHTML), and assemble the other parts (icons, file size) likewise.
• Avoid inserting any values from file.name or file.size via .innerHTML or equivalent unsanitized interpolation.
• Only use innerHTML for fixed markup, or not at all.

Implementation plan:

• Replace the block where fileItem.innerHTML is set with code that builds the markup using DOM manipulation (createElement).
• Assemble and append nodes for the file icon, file name (as text), file size (as text).
• Ensure all existing CSS classes and layout remain unchanged.

Required:
Only standard DOM API, no imports or external dependencies.

File/region to change:

• fweb/static/js/FileHandler.js, lines 39–45.

---