Opened 5 years ago
Closed 4 years ago
#8814 closed defect (fixed)
Can't open HTTP URL whose file name contains ".."
Reported by: | Josef Zlomek | Owned by: | Steven Liu |
---|---|---|---|
Priority: | normal | Component: | avformat |
Version: | git-master | Keywords: | http |
Cc: | josef@pex.com, liuqi05@kuaishou.com, Marton Balint | Blocked By: | |
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
Summary of the bug:
FFmpeg can't open HTTP URL whose file name contains ".." as in my example below.
The problem is that the function trim_double_dot_url does not handle ".." inside a name correctly. It should eliminate just directory "..".
How to reproduce:
% ffplay 'https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3?dest-id=152630' ffmpeg version master (4.3.1) built on Linux
Patches should be submitted to the ffmpeg-devel mailing list and not this bug tracker.
Backtrace:
#0 trim_double_dot_url (buf=0x7fffe94fb070 "", rel=0x7fffe94fc12a "https://hwcdn.libsyn.com/p/3/a/f/3afabfbd7783eb57/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3?c_id=7"..., size=4096) at libavformat/url.c:83
#1 0x00007ffff73bbae7 in ff_make_absolute_url (buf=0x7fffe94fb070 "", size=4096, base=0x7fffd0004040 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"...,
rel=0x7fffe94fc12a "https://hwcdn.libsyn.com/p/3/a/f/3afabfbd7783eb57/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3?c_id=7"...) at libavformat/url.c:156
#2 0x00007ffff7281ca6 in parse_location (s=0x7fffd00016b0, p=0x7fffe94fc12a "https://hwcdn.libsyn.com/p/3/a/f/3afabfbd7783eb57/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3?c_id=7"...) at libavformat/http.c:679
#3 0x00007ffff7282cf1 in process_line (h=0x7fffd0001570, line=0x7fffe94fc120 "location", line_count=3, new_location=0x7fffe94fd5f8) at libavformat/http.c:1004
#4 0x00007ffff7283672 in http_read_header (h=0x7fffd0001570, new_location=0x7fffe94fd5f8) at libavformat/http.c:1168
#5 0x00007ffff7284287 in http_connect (h=0x7fffd0001570, path=0x7fffe94ffa00 "/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3?dest-id=152630",
local_path=0x7fffe94ffa00 "/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3?dest-id=152630", hoststr=0x7fffe9501210 "traffic.libsyn.com", auth=0x7fffe9500e00 "", proxyauth=0x7fffe9500a00 "", new_location=0x7fffe94fd5f8)
at libavformat/http.c:1369
#6 0x00007ffff7280a27 in http_open_cnx_internal (h=0x7fffd0001570, options=0x7fffe9501ce0) at libavformat/http.c:253
#7 0x00007ffff7280ab7 in http_open_cnx (h=0x7fffd0001570, options=0x7fffe9501ce0) at libavformat/http.c:273
#8 0x00007ffff728193d in http_open (h=0x7fffd0001570, uri=0x7fffd00015d0 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"..., flags=1, options=0x7fffe9501ce0) at libavformat/http.c:590
#9 0x00007ffff7221005 in ffurl_connect (uc=0x7fffd0001570, options=0x7fffe9501ce0) at libavformat/avio.c:210
#10 0x00007ffff72216f4 in ffurl_open_whitelist (puc=0x7fffe9501bc0, filename=0x7fdb40 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"..., flags=1, int_cb=0x7fffd0001008, options=0x7fffe9501ce0,
whitelist=0x0, blacklist=0x0, parent=0x0) at libavformat/avio.c:348
#11 0x00007ffff722545b in ffio_open_whitelist (s=0x7fffd0000b60, filename=0x7fdb40 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"..., flags=1, int_cb=0x7fffd0001008, options=0x7fffe9501ce0, whitelist=0x0,
blacklist=0x0) at libavformat/aviobuf.c:1142
#12 0x00007ffff7345e5e in io_open_default (s=0x7fffd0000b40, pb=0x7fffd0000b60, url=0x7fdb40 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"..., flags=1, options=0x7fffe9501ce0) at libavformat/options.c:188
#13 0x00007ffff73bd22d in init_input (s=0x7fffd0000b40, filename=0x7fdb40 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"..., options=0x7fffe9501ce0) at libavformat/utils.c:432
#14 0x00007ffff73bd740 in avformat_open_input (ps=0x7fffe9501e20, filename=0x7fdb40 "https://traffic.libsyn.com/secure/radicalpersonalfinance/2020-07-20_Friday_QA-Equity_Stripping_a_House_in_Divorce_Buy_Whole_Life_Insurance_in_a_Sabbatical_Affirmations_in_Personal_Development_Etc..mp3"..., fmt=0x0, options=0x620bc8 <format_opts>) at libavformat/utils.c:569
#15 0x000000000041615f in read_thread (arg=0x1e0c980) at fftools/ffplay.c:2792
#16 0x00007ffff457e8dc in ?? () from /usr/lib64/libSDL2-2.0.so.0
#17 0x00007ffff45f31d9 in ?? () from /usr/lib64/libSDL2-2.0.so.0
#18 0x00007ffff42ff4f9 in start_thread () from /lib64/libpthread.so.0
#19 0x00007ffff4037f2f in clone () from /lib64/libc.so.6
Change History (11)
comment:1 by , 5 years ago
Cc: | added |
---|---|
Owner: | set to |
Status: | new → open |
follow-up: 3 comment:2 by , 5 years ago
I think this would not work if the file name ends with "..", for example "xyz.."
I guess we need to test if ".." is at the beginning of the string or after "/",
and ".." is at the end of the string or before "/".
comment:3 by , 5 years ago
Replying to jzlomek:
I think this would not work if the file name ends with "..", for example "xyz.."
I guess we need to test if ".." is at the beginning of the string or after "/",
and ".." is at the end of the string or before "/".
Just try it please.
i think it should work, because just check the .. is directory or file.
but i think it maybe have safety problem, i will check there have next byte by strlen(node).
follow-up: 5 comment:4 by , 5 years ago
The patchset does not wrok well, as I suggested before.
Testcase:
test("http://server/foo/bar", "aa/bb/cc/dd/ee../other/url/a.mp3");
gives:
http://server/foo/bar aa/bb/cc/dd/ee../other/url/a.mp3 => http://server/foo/aa/bb/cc/dd/other/url/a.mp3
but should be
http://server/foo/bar aa/bb/cc/dd/ee../other/url/a.mp3 => http://server/foo/aa/bb/cc/dd/ee../other/url/a.mp3
comment:5 by , 5 years ago
Replying to jzlomek:
The patchset does not wrok well, as I suggested before.
Testcase:
test("http://server/foo/bar", "aa/bb/cc/dd/ee../other/url/a.mp3");
gives:
http://server/foo/bar aa/bb/cc/dd/ee../other/url/a.mp3 => http://server/foo/aa/bb/cc/dd/other/url/a.mp3
but should be
http://server/foo/bar aa/bb/cc/dd/ee../other/url/a.mp3 => http://server/foo/aa/bb/cc/dd/ee../other/url/a.mp3
try this patch please:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1853
follow-up: 7 comment:6 by , 5 years ago
It is better but still not correct. Consider this test:
test("http://server/foo/bar", "a/b/../c/d/../e../.../..f/g../h../other/url/a.mp3/...");
It should give "http://server/foo/bar/a/c/e../.../..f/g../h../other/url/a.mp3/...".
I think the best would be to use strtok(p, "/") to split the path into the components and for each ".." component remove the previous one (if there are some still).
comment:7 by , 5 years ago
Replying to jzlomek:
It is better but still not correct. Consider this test:
test("http://server/foo/bar", "a/b/../c/d/../e../.../..f/g../h../other/url/a.mp3/...");
It should give "http://server/foo/bar/a/c/e../.../..f/g../h../other/url/a.mp3/...".
I think the best would be to use strtok(p, "/") to split the path into the components and for each ".." component remove the previous one (if there are some still).
What about this one:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1871
comment:8 by , 5 years ago
PAtchset https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1871
does not work:
- /foo/bar .. => /
- /foo/bar/baz .. => /foo/
+ /foo/bar .. => foo/bar/baz
+ /foo/bar/baz .. => /foofoo/bar/baz
Also this would be nice:
- http://server/foo/bar a/b/../c/d/../e../..f/.../other/url/.. => http://server/foo/a/c/e../..f/.../other/
+ http://server/foo/bar a/b/../c/d/../e../..f/.../other/url/.. => http://server/foo/a/c/e../..f/.../other
comment:9 by , 5 years ago
However, my patchset works well:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1870
comment:10 by , 5 years ago
Keywords: | http added; URL removed |
---|
comment:11 by , 4 years ago
Cc: | added |
---|---|
Resolution: | → fixed |
Status: | open → closed |
Should be fixed in 1201687da268c11459891a80ca1972aeaca8db88.
try this patchset please:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1850