Skip to content

fix: validate FDP-supplied ids before building request URIs (SSRF) - #62

Open
qplevier wants to merge 3 commits into
masterfrom
fix/ssrf-fdpclient-id-validation
Open

fix: validate FDP-supplied ids before building request URIs (SSRF)#62
qplevier wants to merge 3 commits into
masterfrom
fix/ssrf-fdpclient-id-validation

Conversation

@qplevier

@qplevier qplevier commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Snyk Code flagged a CWE-918 SSRF issue: task.uuid, task.UUID, and resourceId — values returned by the remote FDP server — were concatenated directly into outgoing request URIs in FdpClient.
  • Adds a validatedId helper that requires these ids to be well-formed UUIDs before use, so a malicious/compromised FDP response can't inject extra path segments or redirect requests elsewhere.
  • Applied at all four affected call sites: updateSchema, releaseSchema, fetchResource, updateResource.

🤖 Generated with Claude Code

task.uuid, task.UUID, and resourceId come from FDP server responses and
were concatenated directly into outgoing request URIs. A malicious or
compromised FDP instance could inject extra path segments or redirect
the request elsewhere (CWE-918). Require these ids to be well-formed
UUIDs before use.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 3, 2026 11:43
@kburger

kburger commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens FdpClient against CWE-918-style URI construction issues by validating FDP-supplied identifiers before embedding them into outgoing request URIs.

Changes:

  • Added a validatedId(String) helper that requires well-formed UUIDs before use in request paths.
  • Applied UUID validation at the affected URI construction sites (updateSchema, releaseSchema, fetchResource, updateResource).
Suppressed comments (1)

src/main/java/nl/healthri/fdp/uploadschema/integrations/FdpClient.java:284

  • fetchResource() logs the FDP-supplied resourceId before it is validated. If the id is malicious/compromised, this can still cause log injection (newlines/control chars) even though URI building now validates the id. Log the validated id instead (or avoid logging untrusted ids).
            URI uri = new URI(this.hostname + "/resource-definitions/" + validatedId(resourceId));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 3, 2026 11:50
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/main/java/nl/healthri/fdp/uploadschema/integrations/FdpClient.java:68

  • Catching NullPointerException here is avoidable and makes the intent less clear. A simple explicit null check keeps the exception handling focused on UUID parsing failures and avoids relying on NPE control flow.
    private static String validatedId(String id) {
        try {
            return UUID.fromString(id).toString();
        } catch (IllegalArgumentException | NullPointerException e) {
            String safeId = id == null ? "null" : id.replaceAll("[\\r\\n\\t]", "_");

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.

3 participants