downloads webui page issues webrequests directly |
|||||
Issue descriptionthis happens when the user retries a download
,
Jul 11
@jochen: Is this bug fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1120818? Or are you suggesting that there are more actions to be taken? IIUC, it is part of Downloads core functionality to be able to spawn a network request somehow (directly or indirectly). What are the alternatives here?
,
Jul 11
is this bug also fixed after crrev.com/c/1120818 (which fixed bug 857441 )?
,
Jul 11
the webrequest extension api has a check that webui pages don't access web resources. In that CL, I just changed the way to download is triggered (no idea why it didn't trigger for the old code path either). The comment in the web request API said to file a bug if you need an exception from this check and link it from issue 829412, so I did that. No idea how to fix this
,
Jul 12
Contacted rockot@ who added the check, and sending the request from the Web UI C++ handler like the CL above does should be okay; the check is intended to catch only direct requests from Web UI. However, initializing the DownloadUrlParams with the render process/view ids derived from the Web UI WebContents is causing the request to look like it is coming from the Web UI and trigger the check. Using the 3 parameter constructor at [1] instead seems to prevent the check firing, but there is a warning that it may not be safe to use unless the URL has been previously vetted. It seems like that may be the case here, since the WebUI doesn't send the URL to retry directly, but instead sends a download ID that is matched to an existing downloads item with associated URL by the handler. Maybe someone from the downloads team can comment on whether using the constructor without frame IDs would be safe in this case? [1] https://cs.chromium.org/chromium/src/components/download/public/common/download_url_parameters.h?l=79
,
Jul 12
Thanks Rebekah for the investigation. Min: wdyt about using that constructor in this case? Since this is a retry, is it ok since the security checks must have been done for the initial download?
,
Jul 12
I also think this should be fine. This should be similar to the download resumption case, but instead we are resuming from 0% and have created a new download item. Using the 3 parameter constructor will bypass DownloadRequestLimiter, so it could be abused to trigger a large amount of download without user consent.
,
Jul 12
Fir the 3 parameter constructor, I am not sure if there are extensions that could trigger download retry request on chrome://downloads page. But if this is possible, then the extension could trigger a DOS attack by issuing a lot of retry request, and they won't go thru the DownloadRequestLimiter.
,
Jul 19
extensions shouldn't be able to do stuff on chrome://downloads, and I think users would reasonably expect that they can restart a bunch of downloads on that page in quick succession, so actually bypassing the rate limiting might be ok?
,
Jul 19
per #9, if that's ok, then I think the 3 constructor param ctor should be fine
,
Jul 26
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by erikc...@chromium.org
, Jul 9