Hi On Thu, Apr 28, 2016 at 01:47:27PM +0200, Ingo Schwarze wrote: > Hi Nic, > > Nicholas Marriott wrote on Wed, Apr 27, 2016 at 03:36:25AM -0600: > > > CVSROOT: /cvs > > Module name: src > > Changes by: n...@cvs.openbsd.org 2016/04/27 03:36:25 > > > > Modified files: > > usr.bin/tmux : utf8.c > > > > Log message: > > Loads of platforms appear to have old or broken Unicode character type > > information and are missing widths for relatively common Unicode > > characters (so mbtowc() works, but wcwidth() fails). So if wcwidth() > > returns -1, assume a width of 1 instead of ignoring the character. > > I consider this is a backwards approach to portability. > OpenBSD software ought to be developed to work correctly according > to standards (as long as the standards aren't badly broken, which > isn't the case here). > > If *other* operating systems are broken, that should be dealt with > in portable versions of OpenBSD software, not in OpenBSD itself. > We don't want #ifdef in the code on cvs.openbsd.org to deal with > brokenness in other operating systems. We definitely don't want to > cripple OpenBSD base tools just because other systems are broken. > Due to afresh1@'s good work, we have a good wcwidth(3), and we don't > want to punish OpenBSD users for other system's deficiencies.
I don't agree, this change is small and relatively harmless and we do not need another difference to portable for it. > > This particular change looks not only outright wrong, but even > dangerous. Treating non-printable characters as printable can > cause printing them to the terminal, which may even result in > vulnerabilities if you are unlucky. tmux is not some sort of terminal firewall. Of course we try to avoid anything obviously stupid, but we also want stuff that works outside tmux to also work inside. tmux is /not/ getting the width with wcwidth() for some sort of protective sanity check, it is getting it so it can keep its own state correct. If we can't get the width, using the best guess of what the terminal outside will do is fair and will fix more problems than it causes. Note that we do not accept characters we deem to be invalid or where mbtowc() fails. But assuming a width of 1 where we can't get a width from the system is not a big step and I do not think it poses any more of a security risk than just running the application without tmux. > > I strongly object to code saying > > width = wcwidth(wc); > if (width < 0) > width = 1; > > It's an insecure idiom and should not be used. In particular > not on OpenBSD. I do not agree. > > If you have to support a system where wcwidth(3) is broken, > the safest way is to > > #define wcwidth(c) (iswprint(c) ? 1 : -1) > > on that system only, disabling double-width character support, but > not exposing the system to printability vulnerabilities. You should I think this is a poor idea and will break many working characters for no reason. Not to mention it would be difficult to detect a broken wcwidth() in the first place. > not treat non-printable characters as printable even there. If > iswprint(3) is broken, too, you should not support UTF-8 on that > system at all but strictly limit tmux(1) to 7-bit ASCII for security > reasons. Of course, be careful to correctly treat C0 ASCII control > codes in that case. Part of the reason for using the system locale is to avoid having to support multiple character sets.
