View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0003717 | Far Manager | Editor | public | 2019-06-18 07:49 | 2019-08-08 19:42 |
Reporter | pepak | Assigned To | DrKnS | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Platform | x64 | OS | Windows | OS Version | 7 |
Product Version | 3.0 | ||||
Summary | 0003717: #5411 should be optional | ||||
Description | Update #5411 changed the way an edited file is saved to the disk: Instead of using in-place save, a new file is created and then renamed. Unfortunately, this prevents editing of files unless the user has a DELETE privilege on the original file. FAR resorts to elevation in this case, but that is not a good solution as it leaves FAR in a privileged state afterwards. The original approach didn't need either a DELETE privilege or elevation, it was sufficient if the user had a WRITE privilege. Also, this change causes the original file data to remain on the disk. It is now next to impossible to change the content of a file in such a way the original data is completely destroyed. | ||||
Steps To Reproduce | 1) Create a directory where the user has READ, WRITE and EXECUTE but not DELETE. 2) Create a new text file there. 3) Edit the file in 5411. 4) Save the file. 5) The saving will fail and a new temp file with the saved contents appears. | ||||
Tags | No tags attached. | ||||
Build | 5448 | ||||
|
Here is a patch which achieves just that. Copy to FAR source tree and run 'git apply patch.diff'. patch.diff (3,871 bytes)
From 62a01366c9f00fc54639800a62f6a94042384762 Mon Sep 17 00:00:00 2001 From: pepak <pepak@pepak.net> Date: Thu, 20 Jun 2019 06:01:05 +0200 Subject: [PATCH] Safe saving of editor files (build 5411) now optional --- far/config.cpp | 1 + far/config.hpp | 1 + far/dizlist.cpp | 2 +- far/fileedit.cpp | 2 +- far/usermenu.cpp | 2 +- 5 files changed, 5 insertions(+), 3 deletions(-) diff --git a/far/config.cpp b/far/config.cpp index 15e41c313..27d6a4ad4 100644 --- a/far/config.cpp +++ b/far/config.cpp @@ -1776,6 +1776,7 @@ void Options::InitConfigsData() {FSSF_PRIVATE, NKeyEditor, L"FileSizeLimit"sv, EdOpt.FileSizeLimit, 0}, {FSSF_PRIVATE, NKeyEditor, L"KeepEditorEOL"sv, EdOpt.KeepEOL, true}, {FSSF_PRIVATE, NKeyEditor, L"NewFileUnixEOL"sv, EdOpt.NewFileUnixEOL, false}, + {FSSF_PRIVATE, NKeyEditor, L"SafeSaving"sv, EdOpt.SafeSaving, true}, {FSSF_PRIVATE, NKeyEditor, L"CreateBackups"sv, EdOpt.CreateBackups, false}, {FSSF_PRIVATE, NKeyEditor, L"PersistentBlocks"sv, EdOpt.PersistentBlocks, false}, {FSSF_PRIVATE, NKeyEditor, L"ReadOnlyLock"sv, EdOpt.ReadOnlyLock, 0}, diff --git a/far/config.hpp b/far/config.hpp index efff690e0..78be39c59 100644 --- a/far/config.hpp +++ b/far/config.hpp @@ -522,6 +522,7 @@ public: BoolOption KeepEOL; BoolOption AddUnicodeBOM; BoolOption NewFileUnixEOL; + BoolOption SafeSaving; BoolOption CreateBackups; }; diff --git a/far/dizlist.cpp b/far/dizlist.cpp index d0b4a2b4d..79a3fa4df 100644 --- a/far/dizlist.cpp +++ b/far/dizlist.cpp @@ -342,7 +342,7 @@ bool DizList::Flush(const string& Path,const string* DizName) const auto IsFileExists = FileAttr != INVALID_FILE_ATTRIBUTES; const auto IsHardLink = IsFileExists && GetNumberOfLinks(m_DizFileName) > 1; const auto IsSymLink = IsFileExists && (FileAttr & FILE_ATTRIBUTE_REPARSE_POINT); - const auto SaveSafely = IsFileExists && !IsHardLink && !IsSymLink; + const auto SaveSafely = Global->Opt->EdOpt.SafeSaving && IsFileExists && !IsHardLink && !IsSymLink; const auto OutFileName = SaveSafely? MakeTempInSameDir(m_DizFileName) : m_DizFileName; { diff --git a/far/fileedit.cpp b/far/fileedit.cpp index c4d466243..09201ecf4 100644 --- a/far/fileedit.cpp +++ b/far/fileedit.cpp @@ -1924,7 +1924,7 @@ int FileEditor::SaveFile(const string& Name,int Ask, bool bSaveAs, error_state_e const auto IsFileExists = m_FileAttributes != INVALID_FILE_ATTRIBUTES; const auto IsHardLink = IsFileExists && GetNumberOfLinks(Name) > 1; const auto IsSymLink = IsFileExists && (m_FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT); - const auto SaveSafely = IsFileExists && !IsHardLink && !IsSymLink && !IsStream; + const auto SaveSafely = Global->Opt->EdOpt.SafeSaving && IsFileExists && !IsHardLink && !IsSymLink && !IsStream; const auto OutFileName = SaveSafely? MakeTempInSameDir(Name) : Name; { diff --git a/far/usermenu.cpp b/far/usermenu.cpp index 2e8f28799..752b8b7c2 100644 --- a/far/usermenu.cpp +++ b/far/usermenu.cpp @@ -345,7 +345,7 @@ void UserMenu::SaveMenu(const string& MenuFileName) const const auto IsFileExists = FileAttr != INVALID_FILE_ATTRIBUTES; const auto IsHardLink = IsFileExists && GetNumberOfLinks(MenuFileName) > 1; const auto IsSymLink = IsFileExists && (FileAttr & FILE_ATTRIBUTE_REPARSE_POINT); - const auto SaveSafely = IsFileExists && !IsHardLink && !IsSymLink; + const auto SaveSafely = Global->Opt->EdOpt.SafeSaving && IsFileExists && !IsHardLink && !IsSymLink; const auto OutFileName = SaveSafely? MakeTempInSameDir(MenuFileName) : MenuFileName; { -- 2.21.0.windows.1 |
|
I'm not (yet) convinced that it's needed. > this prevents editing of files unless the user has a DELETE privilege on the original file Is that a common case? > it leaves FAR in a privileged state afterwards not really. > It is now next to impossible to change the content of a file in such a way the original data is completely destroyed. Was it ever guaranteed that WriteFile writes to the same physical sectors? Is it possible at all with modern hardware (e.g. SSD)? |
|
Re: Is that a common case? Does it even matter? As it is a recommended security practice to reduce privileges, FAR should support that scenario. I can understand that someone might consider the threat of a damaged file content to be more serious, and by all means allow for that by making the option for "safe saving" enabled by default, but other's might have different priorities. Re: Was it ever guaranteed It was never guaranteed, but it was likely, or at least possible. Now it is almost guaranteed that the original data will *not* get overwritten. |
|
5448: Editor.SaveSafely |
Date Modified | Username | Field | Change |
---|---|---|---|
2019-06-18 07:49 | pepak | New Issue | |
2019-06-20 05:03 | pepak | File Added: patch.diff | |
2019-06-20 05:03 | pepak | Note Added: 0016525 | |
2019-06-20 19:18 | DrKnS | Note Added: 0016527 | |
2019-06-20 19:32 | pepak | Note Added: 0016528 | |
2019-08-08 19:42 | DrKnS | Assigned To | => DrKnS |
2019-08-08 19:42 | DrKnS | Status | new => closed |
2019-08-08 19:42 | DrKnS | Resolution | open => fixed |
2019-08-08 19:42 | DrKnS | Build | => 5448 |
2019-08-08 19:42 | DrKnS | Note Added: 0016567 |