V7 nightly

[JacksonTheMaster]

opened this to run codeQL

[JacksonTheMaster]

To mitigate the possibility of reflected cross-site scripting introduced by the proxy, the proxy should ensure it does not forward arbitrarily dangerous content from the backend to the client. One effective defense is to perform output encoding/escaping (especially for HTML content types) before writing them to the response. The handler should review the backend's Content-Type header:

• For HTML content (text/html or similar), escape dangerous data before sending to the client using html.EscapeString.
• For non-HTML content (JSON, images, etc.), output encoding may not be necessary, but one may consider blocking or restricting unexpected content types.

To implement this:

• Before copying the response body to w, check the Content-Type response header.
• If the response is text/html (or a recognizable HTML-like mime type), read the body, escape it using html.EscapeString, and then write the escaped string to the client.
• For all other content types, use the existing logic (io.Copy).
• Add the required import "html".
  These code changes should be made in src/api/pluginproxy/socketproxy.go, within the block reading the response and writing it to the HTTP client.

---

The best way to fix this is to validate the identifier argument to ensure it cannot be used to escape the intended directory or manipulate the path in an unintended way. Since the filename is formed as "run" + identifier + ".ssui", we should ensure that identifier does not contain:

• Path separators (/ and \)
• Parent directory references (..)
• Absolute path starts (though the prefix mitigates this, but ambiguity is better removed)

A simple approach: check for the presence of /, \, or .. in the identifier and reject the operation if any are found. Optionally, use a regular expression or allow-list for what a valid identifier should look like (e.g., only alphanumeric and dashes/underscores). This validation should be performed as early as possible: ideally at the beginning of SaveRunfileToDisk. If the check fails, log the error and return an error.

No additional dependencies are required; standard library suffices.

Edit only the SaveRunfileToDisk function in src/steamserverui/gallery/runfilegallery.go, adding a check at the top of the function.

[JacksonTheMaster]

The best way to fix the issue is to validate the identifier before constructing the filename in SaveRunfileToDisk. Since the identifier is expected to represent a runfile "name" or key, we should ensure it is a simple segment, with no path separators (/, \) and no parent directory traversals (..). The validation should reject any identifier containing these characters or sequences, returning an error before proceeding with any file system actions.

To implement the fix, in src/steamserverui/gallery/runfilegallery.go in the SaveRunfileToDisk function, right at the beginning, check the identifier for invalid substrings and return an error if found. This can be done using the standard library strings.Contains. No additional imports beyond those already present are needed.

To fix the issue, the untrusted identifier parameter must be validated so that malicious input cannot result in arbitrary file paths. Since the filename is built as run<identifier>.ssui, reasonable constraints would be: the identifier should not contain any path separators (/ or \), parent directory references (..), or be empty. A stricter approach is to allow only alphanumeric characters, dashes, or underscores in the identifier (allow list). The validation should be done at the beginning of SaveRunfileToDisk(identifier string). If the identifier fails validation, the function should return an error and avoid any filesystem access.

Perform the validation right at the start of SaveRunfileToDisk, before using identifier in any way. Add an appropriate error log and return a user-friendly error if validation fails.

Introduce an internal helper function, e.g., isValidIdentifier, which checks (using either a regular expression or string checks) that the identifier is safe (only allowed characters, not empty).

Adjust imports to include "regexp" if using a regular expression.

---

[JacksonTheMaster]

To fix the problem, we must ensure that the value of RunfileIdentifier (sourced from HTTP input as the identifier field) cannot be used to construct malicious file paths. The safest approach is to validate that this identifier complies with strict criteria before saving files or using it as a component in file paths. This can be done by introducing a validation function IsValidRunfileIdentifier(string) bool that checks the identifier only contains allowed characters—e.g., alphanumeric, dash, and underscore, with no slashes, dots, spaces, or other potentially dangerous characters.

The place to insert validation is in SetRunfileIdentifier in src/config/setters.go, since all assignment to RunfileIdentifier is funneled through this setter. Alternatively, further validation could be placed in the handler or when saving, but the setter is the ideal choke point based on the code flow.

We will:

• Add an isValidRunfileIdentifier function to src/config/setters.go validating the identifier string.
• Call it from SetRunfileIdentifier in addition to the whitespace check.
• Update SetRunfileIdentifier to reject identifiers containing invalid characters, and update the error messages accordingly.
• Optionally, we can log a warning if an invalid value is detected (consistent with error handling style shown).

No new dependencies are needed. We can use regexp (already imported in args.go, but not in setters.go) for robust validation; if not available, a simple manual check using strings can be substituted.

---