FmtV() doesn't handle weird utf8 chars

[kjk]

Seen in #878

When a file name contained unusual Unicode character, FmtV() would fail when it was given as %S and generate some garbage when manually converted to utf8 and given as %s. This is because of vsnprintf. Maybe change to std::vsnprintf or finish up StrFormat and use that instead.

[GitHubRulesOK]

Or like the zip message suggest when opening/saving such a file the user does not use characters in a document name that cause downstream problems with .synctex .zip .cbx etc

[SumatraPeter]

Or like the zip message suggest when opening/saving such a file the user does not use characters in a document name that cause downstream problems with .synctex .zip .cbx etc

Naah, kjk's got it right... Far better to fix it than to tell folks in this day and age that they can't use Unicode chars in filenames despite running Sumatra on a modern OS! 🙂

[GitHubRulesOK]

@SumatraPeter I agree that if easy to work around OS failings, it can be worth it.
My suggestion is to avoid known problem characters that will inevitably raise future issues
If you include &format in a filename it is accepted but since command line calls use it for appending commands it is a potential exploit waiting to happen

[SumatraPeter]

I agree that if easy to work around OS failings, it can be worth it.

kjk will correct me if I'm mistaken, but it doesn't seem to be an OS failing. The docs for vsnprintf clearly state that "A return value of -1 indicates that an encoding error has occurred" (where the exact nature of the "encoding error" is defined by the ISO). So it seems it was simply a case of the original code not accounting for certain Unicode characters that would cause an "encoding error" and result in a return value of -1.

IMO kjk is right to fix it rather than simply debar all such characters which the OS itself allows for filenames.

If you include &format in a filename it is accepted but since command line calls use it for appending commands it is a potential exploit waiting to happen

Not sure what you mean by "potential exploit"? After all, neither MS-DOS nor Windows in all these decades has ever prohibited & in filenames despite the special meaning for command.com/cmd.exe, so it seems silly for Sumatra to do so. If this was such a huge issue as to lead to a "potential exploit", it seems so obvious that surely MS would never even have allowed it in the first place (or fixed it ages ago)?

[GitHubRulesOK]

https://duckduckgo.com/?q=exploit+filename+%26+command&t=ffnt&ia=web

[SumatraPeter]

That seems to point to PowerShell not sanitising .ps1 script filenames before execution? What does a failure on the part of the PS devs have to do with Sumatra? I must confess I still do not see the potential for exploit in Sumatra - after all it opens PDFs etc. with & in the filename just fine? TBH logic dictates to me that in this case MS should fix the bug in PS instead of throwing the baby out with the bathwater and completely blocking the use of & in filenames...

[GitHubRulesOK]

It is not just &
One of the reasons I support SumatraPDF is its simplicity to not run coded files (such as forms and others) however it is not 100% immune to all exploits especially if some external file library execution is inevitable.
Having been previously requested to deal with Virus/Trojan/Repurposing self writing files including PDF structure I am wary of extending some features.

[SumatraPeter]

Of course Sumatra cannot be immune to (or responsible for) exploits targeting any third-party code/external library that it uses. But you know what, I'm not going to indulge in idle speculation here about the program's susceptibility to exploits in this case. As far as I can see Sumatra is not executing anything, and even if it was, debarring certain common characters from filenames sounds ridiculous to me. What's next, Sumatra should refuse to open/save PDF, SMX, VBKM etc. files with such 'problem' characters in their filenames? Again, logic would dictate that if the characters pose such a big issue, Sumatra should sanitize the filenames if required while opening. Beyond this I'll leave it to @kjk to weigh in on whether there's any need to be concerned here.