Re: [dev] Re: [st] [PATCH] Avoid buffer overflows in the case of key-mapped strings.

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Wed, 2 Oct 2013 12:47:16 +0200

On Wed, Oct 02, 2013 at 09:36:27AM +0200, Mark Edgar wrote:
> On Tue, 1 Oct 2013 23:04:16 +0200, Roberto E. Vargas Caballero wrote:
> > I agree here, good catch and good implementation. Could you send
> > a mail with the patch and a proper commit message?
>
> No problem!

I think we could avoid some duplication of the code with:

diff --git a/st.c b/st.c
index a295230..3dccbec 100644
--- a/st.c
+++ b/st.c
_AT_@ -264,7 +264,7 @@ typedef struct {
 typedef struct {
         KeySym k;
         uint mask;
- char s[ESC_BUF_SIZ];
+ char *s;
         /* three valued logic variables: 0 indifferent, 1 on, -1 off */
         signed char appkey; /* application keypad */
         signed char appcursor; /* application cursor */
_AT_@ -3582,7 +3582,7 @@ kpress(XEvent *ev) {
         /* 2. custom keys from config.h */
         if((customkey = kmap(ksym, e->state))) {
                 len = strlen(customkey);
- memcpy(buf, customkey, len);
+ cp = customkey;
         /* 3. hardcoded (overrides X lookup) */
         } else {
                 if(len == 0)
_AT_@ -3603,11 +3603,12 @@ kpress(XEvent *ev) {
 
                 memcpy(cp, xstr, len);
                 len = cp - buf + len;
+ cp = buf;
         }
 
- ttywrite(buf, len);
+ ttywrite(cp, len);
         if(IS_SET(MODE_ECHO))
- techo(buf, len);
+ techo(cp, len);
 }
 
What do you think?

>
> I think it might be a good idea to move
> ttywrite();if(MODE_ECHO)techo() into its own function. It also appears
> in bpress().
>

I agree, because always we call to ttywrite we should check the echo flag,
and there are some places where it is not true (for example in mousereport).
When I added echo mode the only one call was in kpress, but some later
additions broke the mode. Moving the test into ttywrite is the correct
way now that we have 11 calls to it. If you write a patch, please remove the
stupid casting in:

                ttywrite((const char *)data, nitems * format / 8);



-- 
Roberto E. Vargas Caballero
----------------------------
k0ga_AT_shike2.com
http://www.shike2.com
Received on Wed Oct 02 2013 - 12:47:16 CEST

This archive was generated by hypermail 2.3.0 : Wed Oct 02 2013 - 12:48:07 CEST