Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: make http api only accept POST requests #2977

Merged
merged 5 commits into from
Apr 10, 2020

Conversation

achingbrain
Copy link
Member

This is to prevent people maliciously controlling your local node by injecting images into webpages with URLs of API endpoints.

BREAKING CHANGE:

Where we used to accept all and any HTTP methods, now only POST is accepted. The API client will now only send POST requests too.

This is to prevent people maliciously controlling your local node
by injecting images into webpages with URLs of API endpoints.

BREAKING CHANGE:

Where we used to accept all and any HTTP methods, now only POST is
accepted.  The API client will now only send POST requests too.
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

This looks good to me! 👍

@jacobheun
Copy link
Contributor

There are still several calls to api.ndjson that don't include method: 'POST'. Hopefully the new tests will show those failures.

@achingbrain
Copy link
Member Author

http.ndjson calls http.iterator which calls http.fetch which is overridden to only send POST requests so it should be handled.

@jacobheun
Copy link
Contributor

http.ndjson calls http.iterator which calls http.fetch which is overridden to only send POST requests so it should be handled.

I think it would be helpful to just remove the method specification in the existing calls then to clean things up. Having most of them with the declaration and some without is confusing, and if it's already doing a post we should just remove the extraneous code.

@achingbrain
Copy link
Member Author

achingbrain commented Apr 9, 2020

I agree, sort of - I'd much rather have something like:

const res = await api.post(...)

for await (const thing of res.ndjson()) {
  ...
}

Though we've already got a lot of code in the http client that's not being used.

I've suggested having a non-general-purpose http client for this sort of thing here: ipfs/js-ipfs-utils#32

@achingbrain
Copy link
Member Author

url: '/api/v0/bootstrap/list'
})

expect(res.statusCode).to.equal(404)
Copy link
Member

@lidel lidel Apr 9, 2020

Choose a reason for hiding this comment

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

cc #2971

This is not interop test, but for what its worth, go-ipfs 0.5.0-rc1 returns 405:

$ curl -I -X GET http://127.0.0.1:5001/api/v0/id
HTTP/1.1 405 Method Not Allowed
Allow: HEAD
Allow: OPTIONS
Allow: POST
Content-Type: text/plain; charset=utf-8
Vary: Origin
X-Content-Type-Options: nosniff
Date: Thu, 09 Apr 2020 23:48:22 GMT
Content-Length: 25

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the correct code but Hapi does not make this easy:

hapijs/hapi#1534
hapijs/hapi#3238

@achingbrain achingbrain merged commit 943d4a8 into master Apr 10, 2020
@achingbrain achingbrain deleted the fix/make-api-post-only branch April 10, 2020 12:57
@achingbrain achingbrain mentioned this pull request Apr 16, 2020
2 tasks
SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022
* fix: make http api only accept POST requests

This is to prevent people maliciously controlling your local node
by injecting images into webpages with URLs of API endpoints.

BREAKING CHANGE:

Where we used to accept all and any HTTP methods, now only POST is
accepted.  The API client will now only send POST requests too.

* test: add tests to make sure we are post-only

* chore: upgrade ipfs-utils

* fix: return 405 instead of 404 for bad methods

* fix: reject browsers that do not send an origin

Also fixes running interface tests over http in browsers against
js-ipfs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants