Re: [hackers] [st][PATCH] set upper limit for REP escape sequence argument
On Mon, Mar 04, 2024 at 10:24:36PM +0200, Tommi Hirvola wrote: > 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. > Hi again, OK that last part convinced me :). I also tested and looked at the xterm code and it seems to indeed limit certain parameter values to 65535. I pushed the patch. Thanks, > -- > Tommi Hirvola > -- Kind regards, Hiltjo
Re: [hackers] [st][PATCH] set upper limit for REP escape sequence argument
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
Re: [hackers] [st][PATCH] set upper limit for REP escape sequence argument
On Mon, Mar 04, 2024 at 12:56:30PM +0200, Tommi Hirvola wrote: > 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 > > Hi, I'm not sure about it. You could still chain REP sequences and "DoS" it. For untrusted input one should be careful about escape sequences anyway. -- Kind regards, Hiltjo
[hackers] [st][PATCH] set upper limit for REP escape sequence argument
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