Re: [hackers] [st][PATCH] set upper limit for REP escape sequence argument

2024-03-04 Thread Tommi Hirvola
On Mon, Mar 04, 2024 at 01:55:29PM +0100, Hiltjo Posthuma wrote:
> I'm not sure about it. You could still chain REP sequences and "DoS" it.

Fortunately, chained REP sequences can be terminated with ^C. You can
try this by copy-pasting the following line into st and pressing CTRL+C:

$ for i in $(seq $((2147483647/65536))); do printf 'L\033[65535b'; done

This also works if we cat a file containing 'L\033[65535b' x 32768. Note
that ^C doesn't work for printf 'L\033[2147483647b' with (unpatched) st.

> For untrusted input one should be careful about escape sequences anyway.

This seems to be the unfortunate reality for modern terminals.

Personally I use st because I can be reasonably confident that cat'ing
server log files does not lead to code execution or other unexpected
behavior. This "REP DoS issue" (if we can call it that) is the only
problem that I've encountered so far.

Even if this REP thing is not a security issue, I think it is still
important to fix in order to prevent (rare) accidental freezing of st.
xterm seems to similarly limit REP argument to ~55K characters in my
environment.

--
Tommi Hirvola



[hackers] [st][PATCH] set upper limit for REP escape sequence argument

2024-03-04 Thread Tommi Hirvola
Previously, printf 'L\033[2147483647b' would call tputc('L') 2^31 times,
making st unresponsive. This commit allows repeating the last character
at most 65535 times in order to prevent freezing and DoS attacks.
---
 st.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/st.c b/st.c
index 77c3e8a..683493d 100644
--- a/st.c
+++ b/st.c
@@ -1643,7 +1643,7 @@ csihandle(void)
ttywrite(vtiden, strlen(vtiden), 0);
break;
case 'b': /* REP -- if last char is printable print it  more times */
-   DEFAULT(csiescseq.arg[0], 1);
+   LIMIT(csiescseq.arg[0], 1, 65535);
if (term.lastc)
while (csiescseq.arg[0]-- > 0)
tputc(term.lastc);
-- 
2.39.2