mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-08 02:14:43 +00:00
Bug 1074793 - Set more restrictive permissions for downloads in the temporary directory. r=paolo
This commit is contained in:
parent
e230d54989
commit
0278ca314e
@ -618,10 +618,15 @@ BackgroundFileSaver::ProcessStateChange()
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Create the target file, or append to it if we already started writing it.
|
// Create the target file, or append to it if we already started writing it.
|
||||||
|
// The 0600 permissions are used while the file is being downloaded, and for
|
||||||
|
// interrupted downloads. Those may be located in the system temporary
|
||||||
|
// directory, as well as the target directory, and generally have a ".part"
|
||||||
|
// extension. Those part files should never be group or world-writable even
|
||||||
|
// if the umask allows it.
|
||||||
nsCOMPtr<nsIOutputStream> outputStream;
|
nsCOMPtr<nsIOutputStream> outputStream;
|
||||||
rv = NS_NewLocalFileOutputStream(getter_AddRefs(outputStream),
|
rv = NS_NewLocalFileOutputStream(getter_AddRefs(outputStream),
|
||||||
mActualTarget,
|
mActualTarget,
|
||||||
PR_WRONLY | creationIoFlags, 0644);
|
PR_WRONLY | creationIoFlags, 0600);
|
||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
|
|
||||||
outputStream = NS_BufferOutputStream(outputStream, BUFFERED_IO_SIZE);
|
outputStream = NS_BufferOutputStream(outputStream, BUFFERED_IO_SIZE);
|
||||||
|
@ -3326,23 +3326,45 @@ nsDownload::ExecuteDesiredAction()
|
|||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
}
|
}
|
||||||
|
|
||||||
nsresult retVal = NS_OK;
|
nsresult rv = NS_OK;
|
||||||
switch (action) {
|
switch (action) {
|
||||||
case nsIMIMEInfo::saveToDisk:
|
case nsIMIMEInfo::saveToDisk:
|
||||||
// Move the file to the proper location
|
// Move the file to the proper location
|
||||||
retVal = MoveTempToTarget();
|
rv = MoveTempToTarget();
|
||||||
|
if (NS_SUCCEEDED(rv)) {
|
||||||
|
rv = FixTargetPermissions();
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
case nsIMIMEInfo::useHelperApp:
|
case nsIMIMEInfo::useHelperApp:
|
||||||
case nsIMIMEInfo::useSystemDefault:
|
case nsIMIMEInfo::useSystemDefault:
|
||||||
// For these cases we have to move the file to the target location and
|
// For these cases we have to move the file to the target location and
|
||||||
// open with the appropriate application
|
// open with the appropriate application
|
||||||
retVal = OpenWithApplication();
|
rv = OpenWithApplication();
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
return retVal;
|
return rv;
|
||||||
|
}
|
||||||
|
|
||||||
|
nsresult
|
||||||
|
nsDownload::FixTargetPermissions()
|
||||||
|
{
|
||||||
|
nsCOMPtr<nsIFile> target;
|
||||||
|
nsresult rv = GetTargetFile(getter_AddRefs(target));
|
||||||
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
|
|
||||||
|
// Set perms according to umask.
|
||||||
|
nsCOMPtr<nsIPropertyBag2> infoService =
|
||||||
|
do_GetService("@mozilla.org/system-info;1");
|
||||||
|
uint32_t gUserUmask = 0;
|
||||||
|
rv = infoService->GetPropertyAsUint32(NS_LITERAL_STRING("umask"),
|
||||||
|
&gUserUmask);
|
||||||
|
if (NS_SUCCEEDED(rv)) {
|
||||||
|
rv = target->SetPermissions(0666 & ~gUserUmask);
|
||||||
|
}
|
||||||
|
return rv;
|
||||||
}
|
}
|
||||||
|
|
||||||
nsresult
|
nsresult
|
||||||
@ -3368,9 +3390,7 @@ nsDownload::MoveTempToTarget()
|
|||||||
rv = target->GetParent(getter_AddRefs(dir));
|
rv = target->GetParent(getter_AddRefs(dir));
|
||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
rv = mTempFile->MoveTo(dir, fileName);
|
rv = mTempFile->MoveTo(dir, fileName);
|
||||||
NS_ENSURE_SUCCESS(rv, rv);
|
return rv;
|
||||||
|
|
||||||
return NS_OK;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
nsresult
|
nsresult
|
||||||
@ -3385,12 +3405,6 @@ nsDownload::OpenWithApplication()
|
|||||||
rv = MoveTempToTarget();
|
rv = MoveTempToTarget();
|
||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
|
|
||||||
// We do not verify the return value here because, irrespective of success
|
|
||||||
// or failure of the method, the deletion of temp file has to take place, as
|
|
||||||
// per the corresponding preference. But we store this separately as this is
|
|
||||||
// what we ultimately return from this function.
|
|
||||||
nsresult retVal = mMIMEInfo->LaunchWithFile(target);
|
|
||||||
|
|
||||||
bool deleteTempFileOnExit;
|
bool deleteTempFileOnExit;
|
||||||
nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
|
nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
|
||||||
if (!prefs || NS_FAILED(prefs->GetBoolPref(PREF_BH_DELETETEMPFILEONEXIT,
|
if (!prefs || NS_FAILED(prefs->GetBoolPref(PREF_BH_DELETETEMPFILEONEXIT,
|
||||||
@ -3409,6 +3423,10 @@ nsDownload::OpenWithApplication()
|
|||||||
// Always schedule files to be deleted at the end of the private browsing
|
// Always schedule files to be deleted at the end of the private browsing
|
||||||
// mode, regardless of the value of the pref.
|
// mode, regardless of the value of the pref.
|
||||||
if (deleteTempFileOnExit || mPrivate) {
|
if (deleteTempFileOnExit || mPrivate) {
|
||||||
|
|
||||||
|
// Make the tmp file readonly so users won't lose changes.
|
||||||
|
target->SetPermissions(0400);
|
||||||
|
|
||||||
// Use the ExternalHelperAppService to push the temporary file to the list
|
// Use the ExternalHelperAppService to push the temporary file to the list
|
||||||
// of files to be deleted on exit.
|
// of files to be deleted on exit.
|
||||||
nsCOMPtr<nsPIExternalAppLauncher> appLauncher(do_GetService
|
nsCOMPtr<nsPIExternalAppLauncher> appLauncher(do_GetService
|
||||||
@ -3425,7 +3443,7 @@ nsDownload::OpenWithApplication()
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return retVal;
|
return mMIMEInfo->LaunchWithFile(target);
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
void
|
||||||
|
@ -307,6 +307,11 @@ protected:
|
|||||||
*/
|
*/
|
||||||
nsresult MoveTempToTarget();
|
nsresult MoveTempToTarget();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Set the target file permissions to be appropriate.
|
||||||
|
*/
|
||||||
|
nsresult FixTargetPermissions();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Update the start time which also implies the last update time is the same.
|
* Update the start time which also implies the last update time is the same.
|
||||||
*/
|
*/
|
||||||
|
@ -612,15 +612,29 @@ this.DownloadIntegration = {
|
|||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
// Now that the file is completely downloaded, make it accessible by other
|
// The file with the partially downloaded data has restrictive permissions
|
||||||
// users on this system. On Unix, the umask of the process is respected.
|
// that don't allow other users on the system to access it. Now that the
|
||||||
// This call has no effect on Windows.
|
// download is completed, we need to adjust permissions based on whether
|
||||||
|
// this is a permanently downloaded file or a temporary download to be
|
||||||
|
// opened read-only with an external application.
|
||||||
try {
|
try {
|
||||||
yield OS.File.setPermissions(aDownload.target.path,
|
// The following logic to determine whether this is a temporary download
|
||||||
{ unixMode: 0o666 });
|
// is due to the fact that "deleteTempFileOnExit" is false on Mac, where
|
||||||
|
// downloads to be opened with external applications are preserved in
|
||||||
|
// the "Downloads" folder like normal downloads.
|
||||||
|
let isTemporaryDownload =
|
||||||
|
aDownload.launchWhenSucceeded && (aDownload.source.isPrivate ||
|
||||||
|
Services.prefs.getBoolPref("browser.helperApps.deleteTempFileOnExit"));
|
||||||
|
// Permanently downloaded files are made accessible by other users on
|
||||||
|
// this system, while temporary downloads are marked as read-only.
|
||||||
|
let unixMode = isTemporaryDownload ? 0o400 : 0o666;
|
||||||
|
// On Unix, the umask of the process is respected. This call has no
|
||||||
|
// effect on Windows.
|
||||||
|
yield OS.File.setPermissions(aDownload.target.path, { unixMode });
|
||||||
} catch (ex) {
|
} catch (ex) {
|
||||||
// Errors with making the permissions less restrictive should be
|
// We should report errors with making the permissions less restrictive
|
||||||
// reported, but should not prevent the download from completing.
|
// or marking the file as read-only on Unix and Mac, but this should not
|
||||||
|
// prevent the download from completing.
|
||||||
Cu.reportError(ex);
|
Cu.reportError(ex);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -14,6 +14,8 @@
|
|||||||
////////////////////////////////////////////////////////////////////////////////
|
////////////////////////////////////////////////////////////////////////////////
|
||||||
//// Globals
|
//// Globals
|
||||||
|
|
||||||
|
const kDeleteTempFileOnExit = "browser.helperApps.deleteTempFileOnExit";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates and starts a new download, using either DownloadCopySaver or
|
* Creates and starts a new download, using either DownloadCopySaver or
|
||||||
* DownloadLegacySaver based on the current test run.
|
* DownloadLegacySaver based on the current test run.
|
||||||
@ -170,21 +172,55 @@ add_task(function test_basic_tryToKeepPartialData()
|
|||||||
/**
|
/**
|
||||||
* Tests the permissions of the final target file once the download finished.
|
* Tests the permissions of the final target file once the download finished.
|
||||||
*/
|
*/
|
||||||
add_task(function test_basic_unix_permissions()
|
add_task(function test_unix_permissions()
|
||||||
{
|
{
|
||||||
// This test is only executed on Linux and Mac.
|
// This test is only executed on Linux and Mac.
|
||||||
if (Services.appinfo.OS != "Darwin" && Services.appinfo.OS != "Linux") {
|
if (Services.appinfo.OS != "Darwin" && Services.appinfo.OS != "Linux") {
|
||||||
do_print("Skipping test_basic_unix_permissions");
|
do_print("Skipping test.");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let download = yield promiseStartDownload(httpUrl("source.txt"));
|
let launcherPath = getTempFile("app-launcher").path;
|
||||||
yield promiseDownloadStopped(download);
|
|
||||||
|
|
||||||
// The file should readable and writable by everyone, but the restrictions
|
for (let autoDelete of [false, true]) {
|
||||||
// from the umask of the process should still apply.
|
for (let isPrivate of [false, true]) {
|
||||||
do_check_eq((yield OS.File.stat(download.target.path)).unixMode,
|
for (let launchWhenSucceeded of [false, true]) {
|
||||||
0o666 & ~OS.Constants.Sys.umask);
|
do_print("Checking " + JSON.stringify({ autoDelete,
|
||||||
|
isPrivate,
|
||||||
|
launchWhenSucceeded }));
|
||||||
|
|
||||||
|
Services.prefs.setBoolPref(kDeleteTempFileOnExit, autoDelete);
|
||||||
|
|
||||||
|
let download;
|
||||||
|
if (!gUseLegacySaver) {
|
||||||
|
download = yield Downloads.createDownload({
|
||||||
|
source: { url: httpUrl("source.txt"), isPrivate },
|
||||||
|
target: getTempFile(TEST_TARGET_FILE_NAME).path,
|
||||||
|
launchWhenSucceeded,
|
||||||
|
launcherPath,
|
||||||
|
});
|
||||||
|
yield download.start();
|
||||||
|
} else {
|
||||||
|
download = yield promiseStartLegacyDownload(httpUrl("source.txt"), {
|
||||||
|
isPrivate,
|
||||||
|
launchWhenSucceeded,
|
||||||
|
launcherPath: launchWhenSucceeded && launcherPath,
|
||||||
|
});
|
||||||
|
yield promiseDownloadStopped(download);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Temporary downloads should be read-only and not accessible to other
|
||||||
|
// users, while permanently downloaded files should be readable and
|
||||||
|
// writable as specified by the system umask.
|
||||||
|
let isTemporary = launchWhenSucceeded && (autoDelete || isPrivate);
|
||||||
|
do_check_eq((yield OS.File.stat(download.target.path)).unixMode,
|
||||||
|
isTemporary ? 0o400 : (0o666 & ~OS.Constants.Sys.umask));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Clean up the changes to the preference.
|
||||||
|
Services.prefs.clearUserPref(kDeleteTempFileOnExit);
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -774,6 +810,13 @@ add_task(function test_cancel_midway_restart_tryToKeepPartialData_false()
|
|||||||
do_check_false(download.hasPartialData);
|
do_check_false(download.hasPartialData);
|
||||||
do_check_true(yield OS.File.exists(download.target.partFilePath));
|
do_check_true(yield OS.File.exists(download.target.partFilePath));
|
||||||
|
|
||||||
|
// On Unix, verify that the file with the partially downloaded data is not
|
||||||
|
// accessible by other users on the system.
|
||||||
|
if (Services.appinfo.OS == "Darwin" || Services.appinfo.OS == "Linux") {
|
||||||
|
do_check_eq((yield OS.File.stat(download.target.partFilePath)).unixMode,
|
||||||
|
0o600);
|
||||||
|
}
|
||||||
|
|
||||||
yield download.cancel();
|
yield download.cancel();
|
||||||
|
|
||||||
// The ".part" file should be deleted now that the download is canceled.
|
// The ".part" file should be deleted now that the download is canceled.
|
||||||
|
@ -1454,7 +1454,7 @@ nsresult nsExternalAppHandler::SetUpTempFile(nsIChannel * aChannel)
|
|||||||
rv = mTempFile->Append(NS_ConvertUTF8toUTF16(tempLeafName));
|
rv = mTempFile->Append(NS_ConvertUTF8toUTF16(tempLeafName));
|
||||||
// make this file unique!!!
|
// make this file unique!!!
|
||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
rv = mTempFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0644);
|
rv = mTempFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
|
||||||
NS_ENSURE_SUCCESS(rv, rv);
|
NS_ENSURE_SUCCESS(rv, rv);
|
||||||
|
|
||||||
// Now save the temp leaf name, minus the ".part" bit, so we can use it later.
|
// Now save the temp leaf name, minus the ".part" bit, so we can use it later.
|
||||||
@ -2401,7 +2401,7 @@ NS_IMETHODIMP nsExternalAppHandler::LaunchWithApplication(nsIFile * aApplication
|
|||||||
fileToUse->Append(mSuggestedFileName);
|
fileToUse->Append(mSuggestedFileName);
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
nsresult rv = fileToUse->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0644);
|
nsresult rv = fileToUse->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0600);
|
||||||
if(NS_SUCCEEDED(rv)) {
|
if(NS_SUCCEEDED(rv)) {
|
||||||
mFinalFileDestination = do_QueryInterface(fileToUse);
|
mFinalFileDestination = do_QueryInterface(fileToUse);
|
||||||
// launch the progress window now that the user has picked the desired action.
|
// launch the progress window now that the user has picked the desired action.
|
||||||
|
Loading…
Reference in New Issue
Block a user