nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 09:27:59 +00:00
Code Issues Projects Releases Wiki Activity

stripctrl: be more careful with wcwidth.

Browse Source 
Coverity points out that wcwidth is capable of returning a negative
number, which suggests that it's a mistake to pass its return value
unchecked to stripctrl_check_line_limit.

This shouldn't cause a problem _in principle_, because by the time
we're making that call, we should already have ruled out the kind of
dangerous control character that might provoke that return value from
wcwidth. But in practice, I couldn't absolutely guarantee that
everyone's idea of what is or is not a control character agrees in
every detail, so I think Coverity is right to urge caution.

Fixed by calling wcwidth (or its wrapper term_char_width) up front,
and ensuring that any character provoking a negative return value is
included in the 'control characters to sanitise out' branch of the
preceding logic.
This commit is contained in:
Simon Tatham 2019-05-05 09:04:39 +01:00
parent 04b6db55f2
commit 01bcae8c5d
1 changed files with 13 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
stripctrl.c
View File

@ -159,16 +159,19 @@ static inline void stripctrl_check_line_limit(
static inline void stripctrl_locale_put_wc(StripCtrlCharsImpl *scc, wchar_t wc)
{
if (iswprint(wc) || stripctrl_ctrlchar_ok(scc, wc)) {
int width = mk_wcwidth(wc);
if ((iswprint(wc) && width >= 0) || stripctrl_ctrlchar_ok(scc, wc)) {
/* Printable character, or one we're going to let through anyway. */
} else if (scc->substitution) {
wc = scc->substitution;
width = mk_wcwidth(wc);
assert(width >= 0);
} else {
/* No defined substitution, so don't write any output wchar_t. */
return;
}
stripctrl_check_line_limit(scc, wc, mk_wcwidth(wc));
stripctrl_check_line_limit(scc, wc, width);
char outbuf[MB_LEN_MAX];
size_t produced = wcrtomb(outbuf, wc, &scc->mbs_out);
@ -180,15 +183,19 @@ static inline void stripctrl_term_put_wc(
StripCtrlCharsImpl *scc, unsigned long wc)
{
ptrlen prefix = PTRLEN_LITERAL("");
int width = term_char_width(scc->term, wc);
if (!(wc & ~0x9F)) {
if (!(wc & ~0x9F) || width < 0) {
/* This is something the terminal interprets as a control
* character. */
if (!stripctrl_ctrlchar_ok(scc, wc)) {
if (!scc->substitution)
if (!scc->substitution) {
return;
else
} else {
wc = scc->substitution;
width = term_char_width(scc->term, wc);
assert(width >= 0);
}
}
if (wc == '\012') {
@ -200,7 +207,7 @@ static inline void stripctrl_term_put_wc(
}
}
stripctrl_check_line_limit(scc, wc, term_char_width(scc->term, wc));
stripctrl_check_line_limit(scc, wc, width);
if (prefix.len)
put_datapl(scc->bs_out, prefix);