Constraints on logfile filenaming

[Kinsie]

As the recent security scare showed, logfiles can be used to overwrite files anywhere on the PC as they can be logged to any folder under any name. This doesn't strike me as a good idea, and I'd recommend putting some sort of constraint on logfile naming to avoid future security issues taking advantage of the same problem.

Some ideas/possibilities:

• Restrict logfile creation to the same folder as gzdoom.exe.
• Add a prefix or suffix to the specified name, so as to avoid possibly overwriting any pre-existing files.
• Force a .log file extension.

[wildweasel]

[*]Add a prefix or suffix to the specified name, so as to avoid possibly overwriting any pre-existing files.

I'd suggest that said prefix be in a similar format to screenshots, i.e. [logname]-[date]-[start time of log].txt.

[edit] Furthermore, replace [logname] with "gzdoom" if the user just types in "logfile" with no arguments and there's not already a log going.

[AFADoomer]

I think the whole idea of artificially limiting save locations for something that has to be typed in by the player is overkill...

But if you're going to be thorough, the save command can take full paths as well, so is similarly problematic.

[wildweasel]

But if you're going to be thorough, the save command can take full paths as well, so is similarly problematic.

Though that brings to mind such use cases as running GZDoom off an external drive or on a system that does not necessarily have full write access (such as playing off a thumb drive on a public computer). Those would need to be accounted for, too, I would think.

[The Zombie Killer]

Additional food for thought: instead of restricting the paths that can be used, require the file to match certain criteria.

An example could be something like: If the file doesn't exist, the path can be used. Otherwise, it must be empty or begin with something like

Code: Select all

# GZDoom Log File

Alternatively the logfile paths could still be restricted, but the user can specify additional "safe" paths in their .ini file (similar to the [IWADSearch.Directories] and [FileSearch.Directories] sections).

[Rachael]

Ultimately the implementation of something like this has to be platform-specific. For obvious reasons you cannot restrict logfile naming on Linux platforms to the same folder as gzdoom - it has to go to /var/log/gzdoom or something like that, if the program is installed using root credentials.

Then it gets even more complicated - you'll have to configure the package manager to ensure that the application has write access to such a folder.

I have no idea how Apple expects such a thing to be done. My guess is they probably have a system log system that works similarly to Windows' Event Viewer.

However, this is a really good idea and the implementation details do need to be ironed out, in my opinion, so that we can go forward with this.

[Chris]

Ultimately the implementation of something like this has to be platform-specific. For obvious reasons you cannot restrict logfile naming on Linux platforms to the same folder as gzdoom - it has to go to /var/log/gzdoom or something like that, if the program is installed using root credentials.

What about saving log files in the same place as save games and the like? On Linux, this is ~/.config/gzdoom. Screenshots are placed in ~/.config/gzdoom/screenshots, so perhaps log files can go in ~/.config/gzdoom/logs (and the name given to the logfile command would be relative to this, an absolute path would be ignored).

[Rachael]

That could work.

[Chris]

It would also need to disallow '..' in the given path, which could break out of the log directory. Probably symlinks too (though the user would have to purposely create them in the log directory targeting outside of it, so I dunno). Maybe it would be best to simply not allow paths in the logfile name at all.

[Rachael]

Yeah, maybe having logfile check for the presence of any '/' or '\\' character and fail if either are present.

[AFADoomer]

So, dumb question, but what would all of this work actually be intended to keep people from doing that they can't already do while, you know, using their computer?

I mean, yes a player could overwrite something important with a logfile or savegame, but they could just as easily go and delete that same file from their OS and cause the same problem...

[ZippeyKeys12]

Ummm...
I think that's why AFADoomer.

[AFADoomer]

Ummm...
I think that's why AFADoomer.

I'm pretty sure that the main point of the 3.2.4 release was that the attack vector discusssed there was removed. This was done by adding a whitelist mechanism to the MENUDEF DoCommand implementation.

The logfile command is not on the whitelist for DoCommand.

[Chris]

I'm pretty sure that the main point of the 3.2.4 release was that the attack vector discusssed there was removed.

It's still an avenue for damage if some other exploit is found. It's also good to ensure functions are focused on doing what they were made to do, and if log files can be put into their own specific directory (like screenshots and savegames), it's good to ask if it's still worth the risk of letting the logfile command write everywhere the process is allowed to.

[The Zombie Killer]

It would also need to disallow '..' in the given path, which could break out of the log directory.

In software I write where this matters, I usually handle this by writing a path parser, which has some sort of "level" integer. Each directory increases it, while ".." decreases it and "." does nothing. You basically just deny paths that ever obtain a negative value.
So: "./some/folder" would be 2, "./some/../folder" would be 1 and "../some/folder" would also be 1, but because at one point the value is negative (-1, the "..") the path would get rejected.