NorthstarLauncher icon indicating copy to clipboard operation
NorthstarLauncher copied to clipboard

[Safe I/O] Only allow creating files with whitelisted filetypes

Open GeckoEidechse opened this issue 11 months ago • 5 comments

Only allow creating files with whitelisted filetypes to prevent writing .bat or .vb files with Safe I/O that could then be called using some exploit (e.g. #674), we should prevent writing file types that by default are interpreted as executable.

Whitelisting was chosen over blacklisting as we can always extend the list and don't have to worry about potentially forgetting to blacklist a certain filetype.

Of course this doesn't prevent writing a bash script to a .txt file and then somehow getting Windows to interpret it as a batch file.

Reading files is not restricted to filetypes as we are primarily concerned with creating files here.

Supersedes #675

Completely untested atm.

Testing instructions:

(to be extended) Basically write a save I/O file with and without a disallowed file extension and check that it handles it respectively.

GeckoEidechse avatar Mar 22 '24 13:03 GeckoEidechse

I wrote this on a system that is unable to compile Northstar and without having written C++ for like more than half a year, would be surprised if it even compiles xD

GeckoEidechse avatar Mar 22 '24 13:03 GeckoEidechse

@ASpoonPlaysGames is also working on that (https://github.com/R2Northstar/NorthstarLauncher/pull/675), are you aware of that? 😄

Alystrasz avatar Apr 01 '24 23:04 Alystrasz

@ASpoonPlaysGames is also working on that (#675), are you aware of that? 😄

ngl that PR is kinda dead, i've just not had the time to do much lately. Honestly this PR might be preferable to mine.

ASpoonPlaysGames avatar Apr 01 '24 23:04 ASpoonPlaysGames

@GeckoEidechse you created the branch on the main repo so we cannot update it :cry:

Alystrasz avatar May 16 '24 16:05 Alystrasz

This isn't directly related to the PR but I noticed while reviewing that the code immediately below directly locks and unlocks the mutex, but ideally it should be using std::lock_guard at the start of the try catch block so that we don't need to explcitly unlock on every scope exit.

barnabwhy avatar Aug 23 '24 10:08 barnabwhy