Trusting file extention is bad practice

[KJNeko]

Blindly trusting the file extension of various files is considered 'bad practice'. Instead (Or after the inital check) the signature of the file should be used to determine if the file is valid or not. (See EXE info and MSI info (See most recent pdf for 11.0))

Changing https://github.com/Willy-JL/F95Checker/blob/70bc13893580fa27832b3e06919cbaaed88b10aa/modules/callbacks.py#L125-L126 To something along the lines of (Pardon my python skills, I am not a python dev)

if exe.suffix in (".exe", ".msi"):  # Probably windows executable
    with exe.open("rb") as f:
        data = f.read(8)
        if data[:2] == b"MZ" or data == b'\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1':
            mark_exe = True

(Check the signature for MSI since I might have typed it wrong) would prevent files that are labeled with .exe and .msi but are not an executable to be loaded/ran

[Willy-JL]

Sure i guess. If you already give code, why not make a PR tho? You get credit then

[KJNeko]

I don't really care if I get credit or not. I just wanted to inform you that this could be a problem.

[FaceCrap]

@KJNeko Although there's nothing against checking magic numbers, I'm wondering what's the worst that could happen in the case of F95Checker? I mean, if it's not an exe or msi file (although I doubt the latter would likely ever be pointed to seeing as how this isn't a game), but something that mimics to be one, what's to prevent a user from (double)clicking that file directly if F95Checker won't run it? If it's a baddie, a virus checker will probably already have intercepted it, and if that user is dumb enough to not have one, he or she is hosed either way...

[KJNeko]

@FaceCrap

In this situation? Not much!

The only reason I mentioned it is because it's a problem in other programs. And I just personally (as a developer) think that doing the 'lazy' way of things just because it's 'not' a problem isn't very good practice. Someone might get into the habit of doing things the lazy way and thus ignoring the proper way when it's actually required.

Might be a really stupid analogy but personally I would be concerned if I someone aim a air soft gun at their own head. Will it hurt? No. Probably not? But. Still would be even more concerning if they then picked up an actual gun.

Also thanks for the quick fix Willy-JL.

[FaceCrap]

True, although given the expertise @Willy-JL already is showing and the nature of this tool, I doubt he'll be one to fall in that trap ;)

@Willy-JL Given that we now have a check on the magic number, and the possibility that whatever the user points at, isn't an executable... might a popup informing said user be useful in this case?

[KJNeko]

Even I forget best practices sometimes and i've been doing programming since middle school. I even have a paragraph of compiler flags (Seriously. It's horrific to look at, send help) to enable extra warnings/errors just to ensure I stay in spec (C++20). Never hurts just to make sure.

[Willy-JL]

I mean, if it's not an exe or msi file (although I doubt the latter would likely ever be pointed to seeing as how this isn't a game), but something that mimics to be one, what's to prevent a user from (double)clicking that file directly if F95Checker won't run it? If it's a baddie, a virus checker will probably already have intercepted it, and if that user is dumb enough to not have one, he or she is hosed either way...

you have to keep in mind that this is linux. an executable file will be opened only if it is marked as executable. so someone could make a file with .exe extension pretending to be a windows executable (which they can run on linux using wine), but really it is a malicious shell (or batch if youre familiar with windows) script that does nasty things. on linux if it wasnt marked executable it would not execute, it would be opened with the default application or something like a text editor. now that i think about it shell scripts have their own version of "magic numbers" on linux, aka the shebang, and f95checker does also check shebangs, so maybe my example isnt that good. but either way marking as executable is what allows a file to be executed, so if a file is pretending to be and exe but really its not, this prevents it from being marked as executable and therefore being executed.

@Willy-JL Given that we now have a check on the magic number, and the possibility that whatever the user points at, isn't an executable... might a popup informing said user be useful in this case?

well this whole PR only concerns the check that is done only on linux and macos to determine whether the file should be marked as executable. the file will be opened regardless. but this check is what determines if it will be executed, or opened with the default app for it. think of a html webpage. with windows it checks the extension. rename .html to .exe, it stops working. linux is smarter than that, it checks the contents (usually, there are some caveats, but thats beside the point). you cant execute a html file, you open it with an appropriate application. so html files should not be marked executable. so on linux you dont make html files executable, and then double clicking them will open them with the default application. now if you were to rename the html to .exe on linux, it would still open like a .html with the default html application. but without this PR it would have been marked as executable, and then that would have caused issues because a html file cannot be executed directly. all this to say that this PR only makes f95checker do the correct thing in edge cases, and should not (in my opinion) warrant a popup

[Willy-JL]

Released in version 10.1.1