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

nocopy: server does not report error for invalid path in Abspath header #5987

Open
radfish opened this issue Feb 12, 2019 · 1 comment
Open
Labels
kind/bug A bug in existing code (including security flaws) topic/filestore Topic filestore

Comments

@radfish
Copy link

radfish commented Feb 12, 2019

Version information:

go-ipfs version: 0.4.19-dev-7ff5604a3
Repo version: 7
System version: amd64/linux
Golang version: go1.11.5
custom build on top of 6a5a268

Type: bug

Description:

Server should return an error if the path passed in Abspath header is invalid, but it should. Instead the server adds the files to the blockstore, ignoring the nocopy argument, which is bad and confusing behavour.

Invalid path currently means: relative, non-existant, or not reachable from server's home directory, as far as my trials show. Btw, I think relative paths should be accepted if they are relative to the server's home directory, because the paths are already recorded in the filestore as relative to the server's home directory, but that merits a separate feature request issue.

Some error reporting has been added in #4558 but the case of this issue was not covered.

GOOD (good request, good result -- data added to filestore):

$ echo barbarbar > bar
$ curl --trace-ascii -   -F "file=@bar;headers=\"Abspath: $PWD/bar\"" "http://localhost:5001/api/v0/add?recursive=True&nocopy=true"
0000: POST /api/v0/add?recursive=True&nocopy=true HTTP/1.1
0036: Host: localhost:5001
004c: User-Agent: curl/7.63.0
0065: Accept: */*
0072: Content-Length: 233
0087: Content-Type: multipart/form-data; boundary=--------------------
00c7: ----b5baf3f0adbdbeb5
00dd:
0000: --------------------------b5baf3f0adbdbeb5
002c: Content-Disposition: form-data; name="file"; filename="bar"
0069: Content-Type: application/octet-stream
0091: Abspath: /home/redfish/bar
00ad:
00af: barbarbar.
00bb: --------------------------b5baf3f0adbdbeb5--
0000: HTTP/1.1 200 OK
...
{"Name":"bar","Hash":"zb2rhoBuvHfNXgV5n7psEJuJkdpCUGjQVxMeGdBpy2BvRDD85","Size":"10"}

$ ipfs filestore ls zb2rhoBuvHfNXgV5n7psEJuJkdpCUGjQVxMeGdBpy2BvRDD85
zb2rhoBuvHfNXgV5n7psEJuJkdpCUGjQVxMeGdBpy2BvRDD85      10 bar 0
$ ipfs block get zb2rhoBuvHfNXgV5n7psEJuJkdpCUGjQVxMeGdBpy2BvRDD85
barbarbar

BAD (bad request, no error, added to blockstore instead of filestore):

$ echo barbar > bar
$ curl --trace-ascii -   -F "file=@bar;headers=\"Abspath: bad/foo/bar\"" "http://localhost:5001/api/v0/add?recursive=True&nocopy=true"
0000: POST /api/v0/add?recursive=True&nocopy=true HTTP/1.1
0036: Host: localhost:5001
004c: User-Agent: curl/7.63.0
0065: Accept: */*
0072: Content-Length: 224
0087: Content-Type: multipart/form-data; boundary=--------------------
00c7: ----fc7bee169d1273d5
00dd:
0000: --------------------------fc7bee169d1273d5
002c: Content-Disposition: form-data; name="file"; filename="bar"
0069: Content-Type: application/octet-stream
0091: Abspath: bad/foo/bar
00a7:
00a9: barbar.
00b2: --------------------------fc7bee169d1273d5--
0000: HTTP/1.1 200 OK
...
{"Name":"bar","Hash":"zb2rhoDsshmqiuLZU1FRfHCURdbpkegJXZZfV1tUa79E5MjX2","Size":"7"}

$ ipfs filestore ls zb2rhoDsshmqiuLZU1FRfHCURdbpkegJXZZfV1tUa79E5MjX2
blockstore: block not found
$ ipfs block stat zb2rhoDsshmqiuLZU1FRfHCURdbpkegJXZZfV1tUa79E5MjX2
Key: zb2rhoDsshmqiuLZU1FRfHCURdbpkegJXZZfV1tUa79E5MjX2
Size: 7
$ ipfs block get zb2rhoDsshmqiuLZU1FRfHCURdbpkegJXZZfV1tUa79E5MjX2
barbar
@magik6k magik6k added kind/bug A bug in existing code (including security flaws) topic/filestore Topic filestore labels Feb 12, 2019
@Stebalien Stebalien added help wanted Seeking public contribution on this issue and removed help wanted Seeking public contribution on this issue labels May 16, 2019
@Stebalien
Copy link
Member

You may have already had that block, I'm not sure. However, while adding a file with an invalid path still works, trying to get that block now fails.

The issue here is that we don't actually look at the specified file when adding content. Instead, we read the content from the HTTP request.


So, to solve this, someone needs to make https://github.com/ipfs/go-unixfs/blob/master/importer/helpers/dagbuilder.go#L242 check the actual file data. However, doing that without repeatedly re-opening the file could be tricky.

@Stebalien Stebalien mentioned this issue Apr 14, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) topic/filestore Topic filestore
Projects
None yet
Development

No branches or pull requests

3 participants