unescaped pathnames may be printed in error cases

[emaste]

Archive-provided pathnames may appear in error strings - some examples in #1609, and in particular #1609 (comment).

The change in #1609 improved an error message, to address issue #1561. In addition the change switched one case that printed an error string from safe_fprintf to fprintf. That case was switched back to safe_fprintf in #2101. There are however a number of other code paths that print error strings without escaping, via lafe_errc or lafe_warnc (and are completely independent of the changes in #1609).

As @jsonn suggests in #1609 (comment) we may consider escaping these when adding them to an error string.

[kientzle]

Good idea! Should such escaping be ASCII-centric or should it try to accommodate UTF-8?

• In either case, we should escape all C0 control characters (values < 32)
• ASCII-centric would also escape all values > 127, but that would break UTF-8 sequences

[emaste]

Yeah, that's a good question :)

Putting the onus on the consumer (e.g. bsdtar) makes it easier to determine if the escaping should be be context-dependent (e.g. based on LANG), but from a maintainability and security perspective we're better off doing it in the library.

IMO escaping control characters is sufficient, since it's really terminal escapes that we want to avoid and the failure mode of not escaping >127 in a non-UTF-8 context is presumably just printing some garbage characters.

(That said, what does safe_fprintf do today?)

[emaste]

That said, what does safe_fprintf do today?

From the comments in safe_fprintf:

                /* Convert to wide char, test if the wide
                 * char is printable in the current locale. */

[kientzle]

Hmmm..... that seems reasonable enough in the context of immediately printing a string.

If we were to change this to quote pathnames inside of the library when building the error string, I'm not sure such an approach would be appropriate. There, we might be better off just quoting any byte value less than 32.