ipfs / kubo

An IPFS implementation in Go
https://docs.ipfs.tech/how-to/command-line-quick-start/
Other
16.13k stars 3.01k forks source link

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

Open radfish opened 5 years ago

radfish commented 5 years ago

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 6a5a268514bf2671d6a364f0b3be8831f692e600

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
Stebalien commented 5 years ago

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.