Re: [screen-devel] [bug #60030] Screen segfaults by displaying some UTF-8 character combination

[Axel Beckert]

Hi Michael,

thanks for your reply, much appreciated. (Saw your name several times
in git blame of encoding.c, so you seem to have more knowledge of that
uncommented code than all others who have commented (sic!) on this
issue so far. :-)

Michael Schroeder schrieb am Thu, Feb 11, 2021 at 02:20:33PM +0000:
> > Thanks, but this seems to break the actual output.
> 
> But isn't the output broken anyway?

Maybe. I don't know how the Chinese glyphs in the PoC should have
looked like and I can't really tell that good what's just font issues
and what's actually terminal issues. When xterm was found to having
similar issues, I temporarily switched to kitty as terminal emulator
to test that stuff in. (Also can better show combined diacriticals
with giant font sizes and scalable fonts. :-)

> Two years ago, the line
> 
>     c = (c & 255) | (unsigned char)D_rend.font << 8;
> 
> was deleted from RAW_PUTCHAR, but the utf8_handle_comb function in
> encode.c was not adapted.

Sounds like a possible cause for this issue.

> Since then, combining characters cannot have worked.

I haven't sent my mail about that yet, but I've tested especially
combining and stacked combining diacriticals (two in a row) with
screen 4.8.0 in Debian Unstable:

And they worked well _before_ any patching of this issue. With my as
well as with Taviso's patch, stacked combining diacriticals no more
work. One of my (randomly choosen) examples was ASCII letter "e" plus
U+0324 COMBINING DIAERESIS BELOW + U+0312 COMBINING TURNED COMMA
ABOVE. Looks like this (if mail transport didn't kill it :-): "e̤̒"

This works fine with screen 4.8.0-3 in Debian Testing (the one which
crashes, without any of the patches in this thread), but no more with
4.8.0-4 which sports my (flawed, but at least no more crashing) patch.

Will try to send that half-finished mail maybe in a ¾-finished state
after this one. :-)

> Do you have a local patch for this?

I need to check against the patches we apply in Debian on top. (Most
of them have been adopted upstream, but not all.)

Have a look at the patches in
https://salsa.debian.org/debian/screen/-/tree/master/debian/patches

Those starting with 0x to 6x are potentially relevant here, except
52fix_screen_utf8_nfd.patch which is not applied. It was meant to fix

> > https://savannah.gnu.org/bugs/index.php?31336 aka
> > https://bugs.debian.org/600246 but made worse.

but caused a regression elsewhere.

Patches starting with 8x are feature patches (all session handling and
sorting related). (Actually 58-show-encoding-hardstatus.patch is also
a feature patch. I probably should clean up patch names and ordering.
But no more before the next Debian stable release. :-)

Patches starting with 9x are cherry-picked patches from upstream or
patches for CVE issues expected to be fixed upstream very soon, too.

But the only patches which touch encoding.c or ansi.c are
99_CVE-2021-26937.patch (encoding.c) and the disabled
52fix_screen_utf8_nfd.patch (ansi.c).

> But there's also the underlying issue that comb_tofront() moves entries
> between the two lists. So here's a revised patch:

Yay, thanks, will test that later today. Much appreciated!

                Regards, Axel
-- 
 ,''`.  |  Axel Beckert <abe@debian.org>, https://people.debian.org/~abe/
: :' :  |  Debian Developer, ftp.ch.debian.org Admin
`. `'   |  4096R: 2517 B724 C5F6 CA99 5329  6E61 2FF9 CD59 6126 16B5
  `-    |  1024D: F067 EA27 26B9 C3FC 1486  202E C09E 1D89 9593 0EDE