From 59724dc14ec00043602052e5ac0efcf4be242754 Mon Sep 17 00:00:00 2001
From: Jonathan Wilkes <jon.w.wilkes@gmail.com>
Date: Sat, 2 Jan 2021 02:56:28 +0000
Subject: [PATCH] more fixes for warnings, mainly string truncation problems
 with snprintf et al

---
 pd/src/s_loader.c    |  4 ++--
 pd/src/s_main.c      | 12 ++++++++----
 pd/src/s_midi.c      |  5 +++--
 pd/src/s_print.c     | 16 +++++++++++++---
 pd/src/s_utf8.c      |  7 ++++++-
 pd/src/x_interface.c |  4 ++--
 pd/src/x_net.c       |  2 +-
 pd/src/x_text.c      |  5 +++++
 8 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/pd/src/s_loader.c b/pd/src/s_loader.c
index d927daa29..1dc0646a8 100644
--- a/pd/src/s_loader.c
+++ b/pd/src/s_loader.c
@@ -117,7 +117,7 @@ static int sys_do_load_lib(t_canvas *canvas, const char *objectname,
     const char *path)
 {
     char symname[MAXPDSTRING], filename[MAXPDSTRING], dirbuf[MAXPDSTRING],
-        *nameptr, altsymname[MAXPDSTRING];
+        *nameptr;
     const char *classname, *cnameptr;
     void *dlobj;
     t_xxx makeout = NULL;
@@ -159,7 +159,7 @@ static int sys_do_load_lib(t_canvas *canvas, const char *objectname,
     if (hexmunge)
     {
         memmove(symname+6, symname, strlen(symname)+1);
-        strncpy(symname, "setup_", 6);
+        memcpy(symname, "setup_", 6);
     }
     else strcat(symname, "_setup");
 
diff --git a/pd/src/s_main.c b/pd/src/s_main.c
index 2dbc5e906..7603e5d1b 100644
--- a/pd/src/s_main.c
+++ b/pd/src/s_main.c
@@ -368,7 +368,11 @@ int sys_main(int argc, char **argv)
     if (getuid() != geteuid())
     {
         fprintf(stderr, "warning: canceling setuid privilege\n");
-        setuid(getuid());
+        if (setuid(getuid()) < 0)
+        {
+            fprintf(stderr, "error: couldn't cancel setuid privilege");
+            exit(1);
+	}
     }
 #endif  /* _WIN32 */
     pd_init();                                  /* start the message system */
@@ -640,7 +644,7 @@ void sys_findprogdir(char *progname)
             /* complicated layout: lib dir is the one we just stat-ed above */
         sys_libdir = gensym(sbuf2);
             /* gui lives in .../lib/pd-l2ork/bin */
-        strncpy(sbuf2, sbuf, FILENAME_MAX-30);
+        strncpy(sbuf2, sbuf, FILENAME_MAX);
         sbuf[FILENAME_MAX-30] = 0;
         strcat(sbuf2, "/lib/pd-l2ork/bin");
         sys_guidir = gensym(sbuf2);
@@ -649,8 +653,8 @@ void sys_findprogdir(char *progname)
     {
             /* simple layout: lib dir is the parent */
             /* gui lives in .../bin */
-        strncpy(sbuf2, sbuf, FILENAME_MAX-30);
-        strncpy(appbuf, sbuf, FILENAME_MAX-30);
+        strncpy(sbuf2, sbuf, FILENAME_MAX);
+        strncpy(appbuf, sbuf, FILENAME_MAX);
         sbuf[FILENAME_MAX-30] = 0;
         sys_libdir = gensym(sbuf);
         strcat(sbuf2, "/bin");
diff --git a/pd/src/s_midi.c b/pd/src/s_midi.c
index 9c9b04f2d..5d6886e26 100644
--- a/pd/src/s_midi.c
+++ b/pd/src/s_midi.c
@@ -822,7 +822,9 @@ void glob_midi_dialog(t_pd *dummy, t_symbol *s, int argc, t_atom *argv)
 {
     int i, nindev, noutdev;
     int newmidiindev[10], newmidioutdev[10];
+#ifdef USEAPI_ALSA
     int alsadevin, alsadevout;
+#endif
 
     for (i = 0; i < 10; i++)
     {
@@ -846,10 +848,9 @@ void glob_midi_dialog(t_pd *dummy, t_symbol *s, int argc, t_atom *argv)
             noutdev++;
         }
     }
+#ifdef USEAPI_ALSA
     alsadevin = atom_getintarg(20, argc, argv);
     alsadevout = atom_getintarg(21, argc, argv);
-        
-#ifdef USEAPI_ALSA
             /* invent a story so that saving/recalling "settings" will
             be able to restore the number of devices.  ALSA MIDI handling
             uses its own set of variables.  LATER figure out how to get
diff --git a/pd/src/s_print.c b/pd/src/s_print.c
index 156f45ac6..5c6705b9d 100644
--- a/pd/src/s_print.c
+++ b/pd/src/s_print.c
@@ -77,13 +77,23 @@ static void doerror(const void *object, const char *s)
 
 static void dologpost(const void *object, const int level, const char *s)
 {
-    char upbuf[MAXPDSTRING];
-    upbuf[MAXPDSTRING-1]=0;
+    /* 1. s is at most MAXPDSTRING, but we're prepending a stupid header
+       below. So for sanity, we first overallocate here to ensure the stupid
+       header doesn't end up overflowing the buffer. */
+    char upbuf[MAXPDSTRING * 2];
 
     // what about sys_printhook_verbose ?
     if (sys_printhook) 
     {
-        snprintf(upbuf, MAXPDSTRING-1, "verbose(%d): %s", level, s);
+        /* 2. The "n" in snprintf stands for "evil": we have to subtract one
+           from total size so the null doesn't get truncated */ 
+        snprintf(upbuf, MAXPDSTRING * 2 - 1, "verbose(%d): %s", level, s);
+        /* 3. Finally, we add a null at MAXPDSTRING-1 so that we end up with
+           a string that fits inside MAXPDSTRING for use with t_symbol, etc.
+
+           If anyone knows how I was *supposed* to do this safely within the
+           constraints of C's stupid stdlib, please teach me... */
+        upbuf[MAXPDSTRING-1]=0;
         (*sys_printhook)(upbuf);
     }
     else if (sys_printtostderr) 
diff --git a/pd/src/s_utf8.c b/pd/src/s_utf8.c
index 8cf20aa27..09d0da32e 100644
--- a/pd/src/s_utf8.c
+++ b/pd/src/s_utf8.c
@@ -80,10 +80,15 @@ int u8_utf8toucs2(uint16_t *dest, int sz, char *src, int srcsz)
         }
         ch = 0;
         switch (nb) {
-            /* these fall through deliberately */
+            /* these fall through deliberately, but commenting each explicitly
+               seems to quiet the compiler. If that's not future proof we
+               can just use copy/pasta and add the break statements */
         case 3: ch += (unsigned char)*src++; ch <<= 6;
+                /* fall through */
         case 2: ch += (unsigned char)*src++; ch <<= 6;
+                /* fall through */
         case 1: ch += (unsigned char)*src++; ch <<= 6;
+                /* fall through */
         case 0: ch += (unsigned char)*src++;
         }
         ch -= offsetsFromUTF8[nb];
diff --git a/pd/src/x_interface.c b/pd/src/x_interface.c
index 6753ea626..e498b5bfa 100644
--- a/pd/src/x_interface.c
+++ b/pd/src/x_interface.c
@@ -1655,10 +1655,10 @@ void *abinfo_new(void)
         if(!abframe)
         {
             error("abinfo: only instantiable inside an ab object");
-            x = 0;
+            return (0);
         }
         else
-            x = pd_new(text_class);
+            return pd_new(text_class);
     }
     return (x);
 }
diff --git a/pd/src/x_net.c b/pd/src/x_net.c
index e81ce2450..4ddecf63b 100644
--- a/pd/src/x_net.c
+++ b/pd/src/x_net.c
@@ -303,7 +303,7 @@ static int netsend_dosend(t_netsend *x, int sockfd,
             bp += res;
         }
     }
-    done:
+    /* done: */
     if (!x->x_bin)
     {
         t_freebytes(buf, length);
diff --git a/pd/src/x_text.c b/pd/src/x_text.c
index 3d61d0cf3..4e5c71915 100644
--- a/pd/src/x_text.c
+++ b/pd/src/x_text.c
@@ -348,6 +348,7 @@ t_binbuf *pointertobinbuf(t_pd *x, t_gpointer *gp, t_symbol *s,
 
     /* these are unused; they copy text from this object to and from a text
         field in a scalar. */
+/*
 static void text_define_frompointer(t_text_define *x, t_gpointer *gp,
     t_symbol *s)
 {
@@ -359,7 +360,10 @@ static void text_define_frompointer(t_text_define *x, t_gpointer *gp,
         binbuf_add(x->x_textbuf.b_binbuf, binbuf_getnatom(b), binbuf_getvec(b));
     }
 }
+*/
 
+/* This doesn't seem to be used, either... */
+/*
 static void text_define_topointer(t_text_define *x, t_gpointer *gp, t_symbol *s)
 {
     t_binbuf *b = pointertobinbuf(&x->x_textbuf.b_ob.ob_pd,
@@ -382,6 +386,7 @@ static void text_define_topointer(t_text_define *x, t_gpointer *gp, t_symbol *s)
         }
     }
 }
+*/
 
     /* bang: output a pointer to a struct containing this text */
 void text_define_bang(t_text_define *x)
-- 
GitLab