Re: libtextstyle calls tputs() with (potentially) invalid string

[Bruno Haible]

Hi,

Adriaan de Groot wrote in
<https://lists.gnu.org/archive/html/bug-gettext/2021-12/msg00023.html>:
> 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.

Thanks for the detailed report on this complicated issue. I have now added
a workaround to GNU libtextstyle:
https://git.savannah.gnu.org/gitweb/?p=gettext.git;a=commitdiff;h=4a66b793721ec4f111b8764b984d407468c1be56

Sorry for the delay.

Bruno