From 01bcae8c5df3d2780dde0996e55699cd25cd4504 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 5 May 2019 09:04:39 +0100
Subject: [PATCH] stripctrl: be more careful with wcwidth.

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.
---
 stripctrl.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/stripctrl.c b/stripctrl.c
index a6883014..b5a8f05f 100644
--- a/stripctrl.c
+++ b/stripctrl.c
@@ -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);