`fs.open` leaks the path to the computer data directory from within CraftOS through error message
MCJack123 opened this issue · 5 comments
In some errors that occur in fs.open
, the error message will contain not only the internal path to the file that was attempted to be written to, but the entire absolute path to the file. This could let malicious programs leak personal information like the user's username or operating system.
There are two ways I have found to reproduce this depending on your OS.
- On Windows, call
fs.open("CON", "w")
in the Lua shell. (This name is reserved in Windows, but not other systems.) - On Mac/Linux, call
fs.open(("\195\166"):rep(256), "w")
in the Lua shell. (Windows trims long names automatically, so this code doesn't error there.)
I expect the error message (second value) to be something like "/ææ...: File name too long" or "/CON: The handle is invalid), but instead it also contains the absolute path to the file (in my case on CCEmuX, "/Users/jack/Library/Application Support/ccemux/computer/0/ææ...: File name too long").
This looks like it's related to how Java returns error messages, but I bet it can be trimmed or worked around so it doesn't give programs the environment information.
(Moved from CCEmuX/CCEmuX#144)
Indeed the error message comes from a java.nio.file.FileSystemException. So either we check before creating a file if it has a bad name, or we check if a FileSystemException is thrown and then change the FileName that is printed. After playing around a bit, I came up with this: https://github.com/2000Slash/CC-Tweaked/commit/a962c31a4667d15d710f3cf35fdd7d67b395302f. This won't work on Mac or Linux yet though, because they use forward slashes.
Oh, if it's a FileSystemException
this is much easier. That has methods for getting the original path and message, so one can just do:
catch(FileSystemException e) {
throw localExceptionOf( e.getReason() ) ;
}
Wow that really looks better than mine, but you still have to get the path right?
Honestly, in these cases it's probably fine to assume the path is the same as the input to write
. I guess in this case it should just be throw new FileSystemException(path, e.getReason())
. Not 100% sure - it's been a while since I looked at this - I'd need to prod in an editor.