Any concerns on adding WebContentsImpl::SaveFrame with specified file name?

[Ramin Halavati]

Hello Content Owners,

I am implementing this UI, which asks the user in the file save dialog if they want to save the original pdf, or the modified one.

If they reply with "original", we need to trigger PDF file download and save it to disk, which is currently done in PDFDocumentHelper::SaveUrlAs, which eventually calls WebContents:SaveFrame.

Calling `SaveFrame` will result in asking the file path from the user again. Are there any concerns about adding another `SaveFrame` function that also receives the file path and doesn't ask it from the user?

(The implementation seems quite trivial as as one can call DownloadUrlParameters::set_file_path in  WebContentsImpl::SaveFrameWithHeaders, but the file path comes with a mojo call from the renderer, can that be an issue?)

Thanks,

Ramin

[Charlie Reis]

Thanks for reaching out.  We'll probably want to discuss this in the CSA meeting on Tuesday, once people are back from the break.  It may be ok to add a separate API that doesn't prompt if it can preserve an earlier file path choice, but it would not be ok to accept a file path from the renderer process, which would allow a compromised renderer to write to a location of its choice.

Charlie

--
You received this message because you are subscribed to the Google Groups "content-owners" group.
To unsubscribe from this group and stop receiving emails from it, send an email to content-owner...@chromium.org.
To view this discussion visit https://groups.google.com/a/chromium.org/d/msgid/content-owners/CAORodQgk2Q1tVztgHFZPwKFoRUDB6aL3NW31tcXtEeXHTuO-bw%40mail.gmail.com.

[Ramin Halavati]

+the...@chromium.org, in case I missed anything.

Thank you.

I think this use case requires that a new path would be savable, since the user selects the file path in the PDF Viewer WebUI, and then based on the selection, we want to send the request to the WebContents to save the content to that path.

Just emphasizing that the path is selected by the user in PDF Viewer WebUI, which is a part of the browser UIs, does that have any effect on the security risks?

[Charlie Reis]

Yes, it's necessary to use a path selected by the user via browser UI, so that's good.  The important part is not to pass that path to a renderer process and back (giving the renderer process a chance to modify it), which is what I thought your first email was saying.

Charlie

[Ramin Halavati]

Thank you, but I think the renderer process can be involved. Here is the expected flow:

1. pdf_viewer.ts which is part of the PDF Viewer WebUI shows the file selection dialog to the user.
2. User selects the path.
3. Based on the type of the the selected path, pdf_viewer.ts sends a message to PDF Plugin to save the PDF to the selected path.
4. PDF Plugin sends the request (with the path) to the browser process to save the file.

So as far as I see, if the renderer process that is running the PDF plugin would be compromised, it can send a not-user-selected path to the browser process to save the file, and that's a blocker for this approach. Am I right?

Ramin Halavati

Senior Software Engineer

rhal...@google.com
+49 173 2614796

Google Germany GmbH

Erika-Mann-Straße 33

80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls Sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde. 

     

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

[Charlie Reis]

Correct-- that would not be ok, and we need to find a slightly different flow.

Thanks for the extra details, though.  More specifically, it's the PDF Plugin process which is the concern, and not the PDF Viewer WebUI process.  The WebUI renderer process doesn't run untrustworthy code and can safely show the dialog and manage the path.  But steps 3 and 4 involve sending the path through the PDF Plugin renderer process, which does handle untrustworthy PDF code from the web.  That process can be compromised and should not be able to specify where to write the data to disk.  Is there some way the PDF Viewer WebUI process can convey the path to the browser process directly?

Charlie

[Ramin Halavati]

Thank you, I will look into and loop back if I find a way.

[Lei Zhang]

At the risk of tacking on another small project to yours, we can streamline how PDF saving works and bypass the issue in this discussion. Instead of having the PDF renderer ask the browser to save the original, the PDF renderer can return the PDF data to the PDF Viewer WebUI, and the WebUI can write the blob out to disk.

I still have a prototype of this laying around, but that is a bit out of date. [1] https://crrev.com/c/2231255

On Mon, Jan 6, 2025 at 10:07 PM Ramin Halavati <rhal...@google.com> wrote:

[Ramin Halavati]

Thank you Lei, I thought the same, but since this approach seemed neater, I wanted to ensure first that this is a no go.

Here is my draft for that (crrev.com/c/6110470). I will clean it up and send it for review once the other CLs for storing searchified PDFs land.