(Moving here from https://chromium-review.googlesource.com/c/chromium/src/+/602788/4/chrome/browser/download/download_prefs.cc#92)

Using base::GetTempDir() as the download directory for auto-open files is not safe, and perhaps slightly less importantly, regresses  crbug.com/132240  .

The user's download directory is ...:

... assumed to be safe to store download files. I.e. we assume that a file created in the target folder will by default inherit the right access permissions. This is true for the users' download folder and any folder that they choose intentionally. This assumption doesn't work for /tmp. See the comment at GetTempDir().

    Even though the initial temporary file is created using base::CreateTemporaryFile() -- thus ensuring the right permissions -- BaseFile discards those permissions and uses the default permissions from the containing directory when renaming. This is the correct thing to do for a directory chosen by the user. Not the correct thing for a temporary directory.

    In addition, /tmp doesn't prevent other unprivileged users from enumerating the files stored there. Using it could expose files to other users on the machine in ways that the user doesn't anticipate.

... assumed to be suitable for holding large downloads. I.e. any intentional managed quota assignment is going to treat the downloads folder differently from /tmp. After the user sets a download folder, downloaded files being redirected elsewhere might be unexpected. This is the gist of  crbug.com/132240 .


Overall, I think the downloads folk need to look again at the solution for issue 55567, run it by security and privacy and implement something that addresses the issues raised there. I don't think the code on trunk is safe. Please try to address these by M62 cutoff.


Not restricting view since the concerns above were raised in the public review at https://chromium-review.googlesource.com/c/chromium/src/+/602788 .