Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relative path enables malicious content to be imported or /dev/null #2103

Closed
prb112 opened this issue Mar 17, 2021 · 1 comment
Closed

Relative path enables malicious content to be imported or /dev/null #2103

prb112 opened this issue Mar 17, 2021 · 1 comment
Assignees
Labels
bug Something isn't working security

Comments

@prb112
Copy link
Contributor

prb112 commented Mar 17, 2021

Describe the bug
Relative path enables malicious content to be imported or /dev/null with FileProvider

To Reproduce
Steps to reproduce the behavior:

  1. Execute an Import with relative path, and the job is able to browse the path and import potentially malicious data.

Expected behavior
Reject the invalid path.

Additional context
@lmsurpre identfied

@prb112 prb112 added bug Something isn't working security labels Mar 17, 2021
@prb112 prb112 added this to the Sprint 2021-04 milestone Mar 17, 2021
@prb112 prb112 self-assigned this Mar 17, 2021
prb112 added a commit that referenced this issue Mar 17, 2021
#2103

File-based exports should be organized by subdirectory #2087

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
@lmsurpre
Copy link
Member

I confirmed that relative paths are now prevented in preflight:

{
    "resourceType": "OperationOutcome",
    "id": "7f-0-0-1-d4b3528c-c24c-40e2-be52-55dc0a7a2ac2",
    "issue": [
        {
            "severity": "fatal",
            "code": "invalid",
            "details": {
                "text": "The path is outside the accepted base path"
            },
            "expression": [
                "<empty>"
            ]
        }
    ]
}

I also confirmed that the error is the same whether the path exists or not, so we are not leaking info about what files exist or not on the system (good).

If a user has access to the internal liberty batch endpoints, its still possible to import files outside fileBase, but that definitely seems acceptable to me.

Note: I also tried passing a path like ~/test and ${PWD}/test it interpreted those relative to the fileBase (/tmp/fhir-server/test/~/fakename and /tmp/fhir-server/test/$PWD/fakename respectively) and submitted the job with success.
Based on this, I added the following comment to an existing issue with a recommended update to the PreFlight check:
#2083 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working security
Projects
None yet
Development

No branches or pull requests

2 participants