libtextstyle calls tputs() with (potentially) invalid string

[Adriaan de Groot]

When libtextstyle outputs a hyperlink, on terminals that support hyperlinks, 
it outputs `\e]8;id=<id-part>;<url-part>;` . The id-part and the url-part are 
user-supplied (by the calling program). In a common case where the ID is NULL, 
libtextstyle invents one based on a simple hash. On my system, with bison as 
calling program, the hash is usually  `5b1...` (32 characters).

Libtextstyle outputs the escape-sequence by calling `tputs()` five times. It 
passes e.g. the id-part as a string to `tputs()`. 

This is code from lib/term-ostream.c:

      tputs ("\033]8;id=",           1, out_ch);
      tputs (new_hyperlink->real_id, 1, out_ch);
      tputs (";",                    1, out_ch);

>From the tputs manpage, it seems that only terminal-escape codes should be 
passed in, or return-values from tparm(). Quoting from, e.g., https://
linux.die.net/man/3/tputs :

===
The tputs routine applies padding information to the string str and outputs 
it. The str must be a terminfo string variable or the return value from tparm, 
tgetstr, or tgoto. affcnt is the number of lines affected, or 1 if not 
applicable. putc is a putchar-like routine to which the characters are passed, 
one at a time.
===

It's hard to say how normative that "str must be" clause is. Since the 
hyperlink ID (nor the preceding escape-code, or the following semicolon) isn't 
a string from one of those acceptable sources, this is *potentially* invalid.

There's one case where this really matters: if ncurses is built with BSD-style 
weirdness enabled, and the string starts with digits. See (sorry, I don't have 
a GNU savannah link at hand):

https://github.com/mirror/ncurses/blob/master/ncurses/tinfo/lib_tputs.c#L332

The special case weirdness turns leading digits into delays. The effect is 
that the hyperlinkid from bison (which starts with "5") turns into a 5ms 
delay, followed by the rest of the hash "b1..." (now 31 characters). That can 
be terribly confusing.

I'd suggest using full_write() instead, to avoid any chance of the user-
provided string being interpreted as a special sequence by ncurses. For what 
it's worth, setting an explicit link ID to something like $<50> probably 
triggers strange behavior even without the BSD-compatibility compile flag.


I've written up a downstream bug report at
        https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=260016
and a lengthy blog post
        https://euroquis.nl/freebsd/2021/11/24/bison.html
about bug-hunting this. There's a (trivial, in my opinion) patch attached to 
that bug report.


Cheers,
[ade]

signature.asc
Description: This is a digitally signed message part.