recent strchriscntrl change

[grawity]

Hello,

It seems to me that the recent commit 19d725d "reject C1 control bytes" is misguided. When the terminals work in UTF-8 mode, they aren't interpreting C1 control bytes; they are interpreting C1 control codepoints, and even the example in the commit message uses UTF-8 encoded codepoints (U+009B encoded as 0xC2 0x9B) rather than single bytes. Yet the function filters raw bytes, whenever they happen to appear as part of an UTF-8 sequence.

However, while 0xC2 0x97 is indeed the UTF-8 encoding of the U+0097 control character, 0xC4 0x97 is instead the UTF-8 encoding of the letter ė: not a control character, and not interpreted as one by any UTF-8 capable terminal, but will be blocked by strchriscntrl() regardless. As a result, chfn -f now rejects my last name as invalid.

The function needs to decode input per the locale codepage and check whether the resulting wchar is in the C1 control range.

(If the user uses a UTF-8 locale but an administrator does not, I don't think that is solvable – it sounds like a self-inflicted problem – and best I can suggest is "use cat -v when reading untrusted files".)

[alejandro-colomar]

Cc: @KhaelK138, @hallyn , @ikerexxe

[alejandro-colomar]

Hello,

It seems to me that the recent commit 19d725d "reject C1 control bytes" is misguided. When the terminals work in UTF-8 mode, they aren't interpreting C1 control bytes; they are interpreting C1 control codepoints, and even the example in the commit message uses UTF-8 encoded codepoints (U+009B encoded as 0xC2 0x9B) rather than single bytes. Yet the function filters raw bytes, whenever they happen to appear as part of an UTF-8 sequence.

However, while 0xC2 0x97 is indeed the UTF-8 encoding of the U+0097 control character, 0xC4 0x97 is instead the UTF-8 encoding of the letter ė: not a control character, and not interpreted as one by any UTF-8 capable terminal, but will be blocked by strchriscntrl() regardless.

Hmmm, it's funny how they made it difficult to handle those control characters.

As a result, chfn -f now rejects my last name as invalid.

#1574 (comment)

Heh.

The function needs to decode input per the locale codepage and check whether the resulting wchar is in the C1 control range.

I don't have a strong opinion on whether we should do wide-character handling, or just revert and consider using UTF-8 entirely a self-inflicted problem. I'm leaning towards the latter.

Do you have an opinion?
Do you want to open a PR?

(If the user uses a UTF-8 locale but an administrator does not, I don't think that is solvable – it sounds like a self-inflicted problem – and best I can suggest is "use cat -v when reading untrusted files".)

Since /etc/passwd is something only root can write to, I guess we can assume it's fine. I assume none of the set-uid programs allow a user to modify it, right? I don't remember all of them.

[stoeckmann]

Since /etc/passwd is something only root can write to, I guess we can assume it's fine. I assume none of the set-uid programs allow a user to modify it, right? I don't remember all of them.

There's chfn which updates full name and the GECOS field.

[alejandro-colomar]

Since /etc/passwd is something only root can write to, I guess we can assume it's fine. I assume none of the set-uid programs allow a user to modify it, right? I don't remember all of them.

There's chfn which updates full name and the GECOS field.

Huh; that means an unprivileged user may confuse root by using setting weird characters there.

In retrospective, I wish those fields would be in a different file (not in /etc/passwd).
But that ship sailed a long time ago.

• Should we do proper UTF-8 handling here?

• Or should we allow C1 control characters (which we were doing until recently)?

• Or should we keep the current behavior, which is safe and simple, but prohibits certain valid UTF-8 characters?

• Any other alternative?

[grawity]

Or should we allow C1 control characters (which we were doing until recently)?

Yeah, the point was that bytes in the "C1" range aren't C1 control characters if the system locale is UTF-8...

Though I wonder if any Linux systems still genuinely use a non-UTF-8 locale as their primary locale. (I don't mean C, but for example ru_RU.KOI8-R or zh_TW.EUC-TW.) Some components already enforce UTF-8 internally – e.g. gnome-terminal will outright refuse to run if it is not under a UTF-8 locale – and a loose 0x9B would be safe under gnome-terminal because it merely becomes an invalid UTF-8 sequence (i.e. "�").

I guess there might be these options:

1. All tools enforce valid UTF-8 on input (regardless of system locale!), and reject Unicode codepoints (wchars) which represent either C0 or C1 control characters. (Though for C0 it doesn't matter whether byte filtering or wchar filtering is done, since UTF-8 guarantees those will be identical.)

   The safety of this relies on the expectation that the terminal will always be UTF-8. I think it's already kind of the case elsewhere though... (I've got to check whether gnome-control-center allows users to change their name; it goes through accounts-service for that.)

2. All tools use wchar filtering (reject 'C1' Unicode codepoints) if the caller's current locale is UTF-8, but revert to byte filtering (reject 'C1' bytes) for other locales.

   I'm not sure how good this option is. In KOI8 and CP125x the C1 range contains IBM graphics (non-text so probably fine to reject); for EUC-TW it seems to be empty; unfortunately GBK/GB18030 and Big5 still legitimately use the range for codepoints.

3. chfn refuses UTF-8 completely and requires the GECOS field to be plain ASCII (i.e. <= 0x7F).

   Not a big fan of this option, but at least it would be consistent in rejecting all non-ASCII input, whereas now the user basically has to guess whether particular Unicode input will trigger a "warning: non-ASCII" or whether it will be "error: invalid". IMO better to be consistent than to be selective about which languages to accept.

4. Same as previous, but chfn requires ASCII unless the caller is root (i.e. sudo chfn)? Bit weird but would still prevent unprivileged users from adding weird stuff.

[alejandro-colomar]

Or should we allow C1 control characters (which we were doing until recently)?

Yeah, the point was that bytes in the "C1" range aren't C1 control characters if the system locale is UTF-8...

Though I wonder if any Linux systems still genuinely use a non-UTF-8 locale as their primary locale. (I don't mean C, but for example ru_RU.KOI8-R or zh_TW.EUC-TW.) Some components already enforce UTF-8 internally – e.g. gnome-terminal will outright refuse to run if it is not under a UTF-8 locale – and a loose 0x9B would be safe under gnome-terminal because it merely becomes an invalid UTF-8 sequence (i.e. "�").

I guess there might be these options:

1. All tools enforce valid UTF-8 on input (regardless of system locale!), and reject _Unicode codepoints_ (wchars) which represent either C0 or C1 control characters. (Though for C0 it doesn't matter whether byte filtering or wchar filtering is done, since UTF-8 guarantees those will be identical.)
   The safety of this relies on the expectation that the terminal will always be UTF-8. I think it's already kind of the case elsewhere though... (I've got to check whether gnome-control-center allows users to change their name; it goes through accounts-service for that.)

I wouldn't volunteer for correctly handling all UTF-8. I strongly oppose this alternative.

2. All tools use wchar filtering (reject 'C1' Unicode codepoints) if the caller's current locale is UTF-8, but revert to byte filtering (reject 'C1' bytes) for other locales.
   I'm not sure how good this option is. In KOI8 and CP125x the C1 range contains IBM graphics (non-text so probably fine to reject); for EUC-TW it seems to be empty; unfortunately GBK/GB18030 and Big5 still legitimately use the range for codepoints.

Handling only multi-byte C1 is easier than handling the entirety of UTF-8. However, I also oppose this.

3. `chfn` refuses UTF-8 completely and requires the GECOS field to be plain ASCII (i.e. `<= 0x7F`).
   Not a big fan of this option, but at least it would be consistent in rejecting all non-ASCII input, whereas now the user basically has to guess whether particular Unicode input will trigger a "warning: non-ASCII" or whether it will be "error: invalid". IMO better to be consistent than to be selective about which languages to accept.

I would be happy with this. I suspect users wouldn't be happy, though.

4. Same as previous, but `chfn` requires ASCII _unless_ the caller is root (i.e. `sudo chfn`)? Bit weird but would still prevent unprivileged users from adding weird stuff.

This sounds reasonable. I still expect many users wouldn't be happy, though.

And there's 5.: Allow anything. If root uses UTF-8, too bad; that's a self-inflicted problem. This means essentially reverting the commit.

I think I'm in favor of 5.

[KhaelK138]

@alejandro-colomar yea, that's the issue I spoke about earlier. @grawity's call on codepoints is good, though, so perhaps we could check for two-byte sequences instead. Perhaps a fix could look something like:

inline bool
strchriscntrl(const char *s)
{
    for (; !streq(s, ""); s++) {
        unsigned char c = *s;

        if (iscntrl(c))
            return true;

        // Check for C1 control codepoints (U+0080-U+009F), encoded as 0xC2 followed by 0x80-0x9F for UTF-8 locales
        if (c == 0xC2) {
            unsigned char next = (unsigned char)*(s + 1);
            if (next >= 0x80 && next <= 0x9F)
                return true;
        }
    }

    return false;
}

It would still miss non UTF-8 locales, but that was always the case, and is very rare. Reverting is also a consideration, just to make sure that legitimate names aren't blocked. CVE-2023-29383 for a similar issue did have pretty low severity.

[alejandro-colomar]

@alejandro-colomar yea, that's the issue I spoke about earlier. @grawity's call on codepoints is good, though, so perhaps we could check for two-byte sequences instead. Perhaps a fix could look something like:

inline bool
strchriscntrl(const char *s)
{
for (; !streq(s, ""); s++) {
unsigned char c = *s;

    if (iscntrl(c))
        return true;

    // Check for C1 control codepoints (U+0080-U+009F), encoded as 0xC2 followed by 0x80-0x9F for UTF-8 locales
    if (c == 0xC2) {
        unsigned char next = (unsigned char)*(s + 1);
        if (next >= 0x80 && next <= 0x9F)
            return true;
    }
}

return false;

}

It would still miss non UTF-8 locales, but that was always the case, and is very rare.

TBH, I don't enjoy handling multi-byte sequences. It's a huge can of worms. I very much prefer telling people to not use UTF-8 at all, or do it at their own risk. FWIW, the complexity of the function above is way above what I'd like to have. For single bytes, strchriscntrl() can be written as a one-liner:

#define strchriscntrl(s)  (!!strpbrk(s, CTYPE_CNTRL))

Let's revert the commit and disclaim that if you use UTF-8, you're at risk.