Skip to content

Potential fix for code scanning alert no. 2: Uncontrolled data used in path expression - #51

Draft
jimklimov wants to merge 1 commit into
masterfrom
alert-autofix-2a
Draft

Potential fix for code scanning alert no. 2: Uncontrolled data used in path expression#51
jimklimov wants to merge 1 commit into
masterfrom
alert-autofix-2a

Conversation

@jimklimov

Copy link
Copy Markdown
Member

Potential fix for https://github.com/networkupstools/jNut/security/code-scanning/2

General fix approach: enforce strict validation at the sink boundary (where the path is consumed) so only safe working directories are accepted. Relying on callers to always sanitize is fragile, especially with a generic setParam API.

Best fix here: in jNut/src/main/java/org/networkupstools/jnut/Scanner.java, validate localExecPath in scan() before creating/using File dir. Reject path traversal patterns (..), and require a canonical existing directory. If validation fails, ignore the provided path and fall back to runtime.exec(params) (current behavior when dir == null), preserving functionality while removing unsafe path usage. No behavior change for valid configured paths.

Concretely:

  • Edit the scan() block around current lines 330–334.
  • Add canonical-path-based validation (getCanonicalFile()), reject values containing .., and ensure directory exists/isDirectory.
  • Catch IOException from canonicalization and treat as invalid (dir = null).
  • No new dependency is needed; existing java.io.* imports already cover required classes.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…n path expression

Validate the `dir` value more diligently. Frown upon relative paths going through `..`.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
dir = null;
} else {
File candidate = new File(localExecPath).getCanonicalFile();
if (candidate.exists() && candidate.isDirectory()) {
dir = null;
} else {
File candidate = new File(localExecPath).getCanonicalFile();
if (candidate.exists() && candidate.isDirectory()) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants