Hi Ted,

Ted Unangst wrote on Sun, Jan 13, 2019 at 03:48:05AM -0500:

> By inspection, it appears possible this access will underrun if the
> first character is a %.

OK schwarze@

That said, i see three more buglets here.

 1. If the mailbox filename ends in a percent sign (not saying that
    is particularly sane, but anyway), one byte is skipped too much:

     $ export MAILPATH='/home/schwarze/mailtest\%%msg1%msg2'  
     $ export MAILCHECK=0                                     
     $ echo test > /home/schwarze/mailtest%                  
     $ echo test > /home/schwarze/mailtest%%msg1
    msg2
     $ 

 2. If the mailbox filename is empty (exactly the case you are
    fixing here), there is really no point in checking and
    re-checking stat("", ...) over and over again.  No, it will
    never succeed, hopefully.

 3. If the message is empty, printing an empty message is not
    really a good idea.  Better print the default message.

     $ export MAILPATH='/home/schwarze/mailtest%'            
     $ echo test > /home/schwarze/mailtest        

     $ 

With the additional patch below:

     $ export MAILPATH='/home/schwarze/mailtest\%%msg1%msg2'
     $ export MAILCHECK=0
     $ echo test > /home/schwarze/mailtest%
    msg1%msg2
     $ echo test > /home/schwarze/mailtest%%msg1
     $ export MAILPATH='/home/schwarze/mailtest%'
     $ echo test > /home/schwarze/mailtest
    you have mail in /home/schwarze/mailtest
     $ 

Feel free to either commit the complete patch below or commit
only your part and optionally OK one, two, or three of my additional
changes.

Some kinds of insects are endangered in nature.
Not so in software: bugs whereever you look.  Even in OpenBSD.

Sigh,
  Ingo


> Index: mail.c
> ===================================================================
> RCS file: /cvs/src/bin/ksh/mail.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 mail.c
> --- mail.c    7 Jan 2019 20:50:43 -0000       1.25
> +++ mail.c    13 Jan 2019 08:41:58 -0000
> @@ -128,7 +128,7 @@ mpset(char *mptoparse)
>               /* POSIX/bourne-shell say file%message */
>               for (p = mpath; (mmsg = strchr(p, '%')); ) {
>                       /* a literal percent? (POSIXism) */
> -                     if (mmsg[-1] == '\\') {
> +                     if (mmsg > mpath && mmsg[-1] == '\\') {
>                               /* use memmove() to avoid overlap problems */
>                               memmove(mmsg - 1, mmsg, strlen(mmsg) + 1);
>                               p = mmsg + 1;


Index: mail.c
===================================================================
RCS file: /cvs/src/bin/ksh/mail.c,v
retrieving revision 1.25
diff -u -p -r1.25 mail.c
--- mail.c      7 Jan 2019 20:50:43 -0000       1.25
+++ mail.c      13 Jan 2019 12:37:06 -0000
@@ -128,10 +128,10 @@ mpset(char *mptoparse)
                /* POSIX/bourne-shell say file%message */
                for (p = mpath; (mmsg = strchr(p, '%')); ) {
                        /* a literal percent? (POSIXism) */
-                       if (mmsg[-1] == '\\') {
+                       if (mmsg > mpath && mmsg[-1] == '\\') {
                                /* use memmove() to avoid overlap problems */
                                memmove(mmsg - 1, mmsg, strlen(mmsg) + 1);
-                               p = mmsg + 1;
+                               p = mmsg;
                                continue;
                        }
                        break;
@@ -141,8 +141,11 @@ mpset(char *mptoparse)
                        mmsg = strchr(mpath, '?');
                if (mmsg) {
                        *mmsg = '\0';
-                       mmsg++;
+                       if (*++mmsg == '\0')
+                               mmsg = NULL;
                }
+               if (*mpath == '\0')
+                       continue;
                mbp = mballoc(mpath, mmsg);
                mbp->mb_next = mplist;
                mplist = mbp;

Reply via email to