From d7ccb9dbe6ac31e5535adcacb6bd5764f07f9b8e Mon Sep 17 00:00:00 2001 From: Sojourner Truth <jon.w.wilkes@gmail.com> Date: Wed, 24 Aug 2016 22:01:04 -0400 Subject: [PATCH] port from Pd Vanilla (some of these were already ported): 5a1b58 These bugs were discovered via static code-analysis https://scan.coverity.com/projects/pure-data-pure-data (The CID numbers are the "Coverity IDs" used to identify the various bugs) - Don't free binbuf within glist_readfrombinbuf (CID:74871) - it must be deleted by the caller - Initialize sockaddr_in (CID:74861,74862,74863,74866) - Initialize indelay/outdelay (CID:74864,74865) - Initialize device-arrays (CID:74853,74854,74855,74856) - Closing unneeded sockets (CID:74827) - Initialize sockaddr_in (CID:74860} - Close file-descriptor if the soundfile is unparsable (CID:74822,74823) - Close file-handle in cleanup (CID:74816) - Call va_end() when handling variadic args (CID:74872) - Added missing break (CID:74800) - Prevent NULL-dereferences (CID:74792,74793) - Size checks when copying into string buffers (CID:74844,74845,74846,74847,74849) - Freeing no-more used storage (CID:74832) - Fixed code block (CID:74806) - Initialize sockaddr_in (CID:74860) - Initialize array-values to 0 (CID:74867) - Free allocated memory (CID:74813,74830) - Removed allocation of unused memory - Use pd_free() to free templates (CID:74814) - template_free() does not free the entire structure (it misses the pd_new() part) - initialize variable (CID:77865) - Prevent NULL-pointer dererencing (CID:77864) - Free heap-allocate binbuf after use (CID:74833) --- pd/src/d_soundfile.c | 18 ++++++++++++------ pd/src/g_canvas.c | 4 ++-- pd/src/g_editor.c | 4 +++- pd/src/g_readwrite.c | 9 ++++++--- pd/src/g_template.c | 2 +- pd/src/m_binbuf.c | 18 +++++++++++------- pd/src/m_class.c | 2 +- pd/src/m_pd.c | 1 + pd/src/s_audio.c | 9 +++++++++ pd/src/s_audio_alsa.c | 2 +- pd/src/s_inter.c | 4 ++-- pd/src/s_path.c | 5 +++-- pd/src/u_pdreceive.c | 2 +- pd/src/u_pdsend.c | 2 +- 14 files changed, 54 insertions(+), 28 deletions(-) diff --git a/pd/src/d_soundfile.c b/pd/src/d_soundfile.c index 04a6644eb..285720ea9 100644 --- a/pd/src/d_soundfile.c +++ b/pd/src/d_soundfile.c @@ -407,12 +407,15 @@ int open_soundfile(const char *dirname, const char *filename, int headersize, long skipframes) { char buf[FILENAME_MAX], *bufptr; - int fd; + int fd, sf_fd; fd = open_via_path(dirname, filename, "", buf, &bufptr, FILENAME_MAX, 1); if (fd < 0) return (-1); - else return (open_soundfile_via_fd(fd, headersize, p_bytespersamp, - p_bigendian, p_nchannels, p_bytelimit, skipframes)); + sf_fd = open_soundfile_via_fd(fd, headersize, p_bytespersamp, + p_bigendian, p_nchannels, p_bytelimit, skipframes); + if (sf_fd < 0) + sys_close(fd); + return (sf_fd); } /* open a soundfile, using open_via_canvas(). This is used by readsf~ in @@ -423,12 +426,15 @@ int open_soundfile_via_canvas(t_canvas *canvas, const char *filename, int header long skipframes) { char buf[FILENAME_MAX], *bufptr; - int fd; + int fd, sf_fd; fd = canvas_open(canvas, filename, "", buf, &bufptr, FILENAME_MAX, 1); if (fd < 0) return (-1); - else return (open_soundfile_via_fd(fd, headersize, p_bytespersamp, - p_bigendian, p_nchannels, p_bytelimit, skipframes)); + sf_fd = open_soundfile_via_fd(fd, headersize, p_bytespersamp, + p_bigendian, p_nchannels, p_bytelimit, skipframes); + if (sf_fd < 0) + sys_close(fd); + return (sf_fd); } static void soundfile_xferin_sample(int sfchannels, int nvecs, t_sample **vecs, diff --git a/pd/src/g_canvas.c b/pd/src/g_canvas.c index 0ece98d49..79cb8badb 100644 --- a/pd/src/g_canvas.c +++ b/pd/src/g_canvas.c @@ -646,8 +646,8 @@ void canvas_setbounds(t_canvas *x, int x1, int y1, int x2, int y2) t_symbol *canvas_makebindsym(t_symbol *s) { char buf[MAXPDSTRING]; - strcpy(buf, "pd-"); - strcat(buf, s->s_name); + snprintf(buf, MAXPDSTRING-1, "pd-%s", s->s_name); + buf[MAXPDSTRING-1] = 0; return (gensym(buf)); } diff --git a/pd/src/g_editor.c b/pd/src/g_editor.c index 7d11efdc6..214a7a74c 100644 --- a/pd/src/g_editor.c +++ b/pd/src/g_editor.c @@ -3060,7 +3060,9 @@ void canvas_done_popup(t_canvas *x, t_float which, t_float xpos, } else { - strcpy(namebuf, class_gethelpname(pd_class(&y->g_pd))); + strncpy(namebuf, class_gethelpname(pd_class(&y->g_pd)), + MAXPDSTRING-1); + namebuf[MAXPDSTRING-1] = 0; dir = class_gethelpdir(pd_class(&y->g_pd)); } if (strlen(namebuf) < 4 || diff --git a/pd/src/g_readwrite.c b/pd/src/g_readwrite.c index dd2f5ef7e..8258f3076 100644 --- a/pd/src/g_readwrite.c +++ b/pd/src/g_readwrite.c @@ -190,7 +190,6 @@ void glist_readfrombinbuf(t_glist *x, t_binbuf *b, char *filename, int selectem) strcmp(vec[message].a_w.w_symbol->s_name, "data")) { pd_error(x, "%s: file apparently of wrong type", filename); - binbuf_free(b); return; } /* read in templates and check for consistency */ @@ -202,7 +201,10 @@ void glist_readfrombinbuf(t_glist *x, t_binbuf *b, char *filename, int selectem) int ntemplateargs = 0, newnargs; nline = canvas_scanbinbuf(natoms, vec, &message, &nextmsg); if (nline < 2) + { + t_freebytes(templateargs, sizeof(*templateargs) * ntemplateargs); break; + } else if (nline > 2) canvas_readerror(natoms, vec, message, nline, "extra items ignored"); @@ -243,10 +245,10 @@ void glist_readfrombinbuf(t_glist *x, t_binbuf *b, char *filename, int selectem) { error("%s: template doesn't match current one", templatesym->s_name); - template_free(newtemplate); + pd_free(&newtemplate->t_pdobj); return; } - template_free(newtemplate); + pd_free(&newtemplate->t_pdobj); } while (nextmsg < natoms) { @@ -608,6 +610,7 @@ t_binbuf *glist_writetobinbuf(t_glist *x, int wholething) ((t_scalar *)y)->sc_vec, b, 0); } } + t_freebytes(templatevec, ntemplates * sizeof(*templatevec)); return (b); } diff --git a/pd/src/g_template.c b/pd/src/g_template.c index 5e8c8d397..8c57631ec 100644 --- a/pd/src/g_template.c +++ b/pd/src/g_template.c @@ -745,7 +745,7 @@ static void *gtemplate_new(t_symbol *s, int argc, t_atom *argv) { t_symbol *sym = atom_getsymbolarg(0, argc, argv); if (argc >= 1) - argc--; argv++; + argc--, argv++; if (gtemplate_cancreate(sym, argc, argv)) { return (gtemplate_donew(canvas_makebindsym(sym), argc, argv)); diff --git a/pd/src/m_binbuf.c b/pd/src/m_binbuf.c index f799c6add..69d5c3977 100644 --- a/pd/src/m_binbuf.c +++ b/pd/src/m_binbuf.c @@ -417,6 +417,7 @@ void binbuf_addbinbuf(t_binbuf *x, t_binbuf *y) } binbuf_add(x, z->b_n, z->b_vec); + binbuf_free(z); //fprintf(stderr,"done binbuf_addbinbuf\n"); } @@ -948,10 +949,11 @@ int binbuf_read(t_binbuf *b, char *filename, char *dirname, int crflag) char *buf; char namebuf[MAXPDSTRING]; - namebuf[0] = 0; if (*dirname) - strcat(namebuf, dirname), strcat(namebuf, "/"); - strcat(namebuf, filename); + snprintf(namebuf, MAXPDSTRING-1, "%s/%s", dirname, filename); + else + snprintf(namebuf, MAXPDSTRING-1, "%s", filename); + namebuf[MAXPDSTRING-1] = 0; if ((fd = binbuf_doopen(namebuf, 0)) < 0) { @@ -1043,10 +1045,12 @@ int binbuf_write(t_binbuf *x, char *filename, char *dir, int crflag) int indx, deleteit = 0; int ncolumn = 0; - fbuf[0] = 0; if (*dir) - strcat(fbuf, dir), strcat(fbuf, "/"); - strcat(fbuf, filename); + snprintf(fbuf, MAXPDSTRING-1, "%s/%s", dir, filename); + else + snprintf(fbuf, MAXPDSTRING-1, "%s", filename); + fbuf[MAXPDSTRING-1] = 0; + if (!strcmp(filename + strlen(filename) - 4, ".pat") || !strcmp(filename + strlen(filename) - 4, ".mxt")) { @@ -1136,7 +1140,7 @@ static t_binbuf *binbuf_convert(t_binbuf *oldb, int maxtopd) { t_binbuf *newb = binbuf_new(); t_atom *vec = oldb->b_vec; - t_int n = oldb->b_n, nextindex, stackdepth = 0, stack[MAXSTACK], + t_int n = oldb->b_n, nextindex, stackdepth = 0, stack[MAXSTACK] = {0}, nobj = 0, i, gotfontsize = 0; t_atom outmess[MAXSTACK], *nextmess; t_float fontsize = 10; diff --git a/pd/src/m_class.c b/pd/src/m_class.c index 5678a92e3..3db693bbb 100644 --- a/pd/src/m_class.c +++ b/pd/src/m_class.c @@ -404,9 +404,9 @@ void class_addmethod(t_class *c, t_method fn, t_symbol *sel, if (argtype != A_NULL) error("%s_%s: only 5 arguments are typecheckable; use A_GIMME", c->c_name->s_name, sel->s_name); - va_end(ap); m->me_arg[nargs] = A_NULL; } + va_end(ap); return; phooey: bug("class_addmethod: %s_%s: bad argument types\n", diff --git a/pd/src/m_pd.c b/pd/src/m_pd.c index a8811ec49..4732f7cf8 100644 --- a/pd/src/m_pd.c +++ b/pd/src/m_pd.c @@ -451,6 +451,7 @@ static t_symbol *midi_gensym(const char *prefix, const char *name) char buf[80]; strncpy(buf, prefix, 79); buf[79] = 0; + buf[79] = 0; strncat(buf, name, 79 - strlen(buf)); return (gensym(buf)); } diff --git a/pd/src/s_audio.c b/pd/src/s_audio.c index 14e113ed5..d535928f3 100644 --- a/pd/src/s_audio.c +++ b/pd/src/s_audio.c @@ -229,6 +229,15 @@ void sys_set_audio_settings(int naudioindev, int *audioindev, int nchindev, char indevlist[MAXNDEV*DEVDESCSIZE], outdevlist[MAXNDEV*DEVDESCSIZE]; int indevs = 0, outdevs = 0, canmulti = 0, cancallback = 0; + + /* initialize device-arrays */ + for(i=0; i<MAXAUDIOINDEV; i++) + realindev[i] = realinchans[i] = 0; + for(i=0; i<MAXAUDIOOUTDEV; i++) + realoutdev[i] = realoutchans[i] = 0; + for(i=0; i<MAXNDEV*DEVDESCSIZE; i++) + indevlist[i] = outdevlist[i] = 0; + audio_getdevs(indevlist, &indevs, outdevlist, &outdevs, &canmulti, &cancallback, MAXNDEV, DEVDESCSIZE); diff --git a/pd/src/s_audio_alsa.c b/pd/src/s_audio_alsa.c index 06e889cda..4726d004e 100644 --- a/pd/src/s_audio_alsa.c +++ b/pd/src/s_audio_alsa.c @@ -617,7 +617,7 @@ int alsa_send_dacs(void) void alsa_printstate( void) { int result, iodev = 0; - snd_pcm_sframes_t indelay, outdelay; + snd_pcm_sframes_t indelay = 0, outdelay = 0; if (sys_audioapi != API_ALSA) { error("restart-audio: implemented for ALSA only."); diff --git a/pd/src/s_inter.c b/pd/src/s_inter.c index 896833d4a..85e9b3ddb 100644 --- a/pd/src/s_inter.c +++ b/pd/src/s_inter.c @@ -1135,7 +1135,7 @@ int sys_startgui(const char *guidir) { pid_t childpid; char cmdbuf[4*MAXPDSTRING]; - struct sockaddr_in server; + struct sockaddr_in server = {0}; int len = sizeof(server); int ntry = 0, portno = FIRSTPORTNUM; int xsock = -1; @@ -1195,7 +1195,7 @@ int sys_startgui(const char *guidir) } else if (sys_guisetportnumber) /* GUI exists and sent us a port number */ { - struct sockaddr_in server; + struct sockaddr_in server = {0}; struct hostent *hp; /* create a socket */ sys_guisock = socket(AF_INET, SOCK_STREAM, 0); diff --git a/pd/src/s_path.c b/pd/src/s_path.c index a1ef27921..04ec7aa8c 100644 --- a/pd/src/s_path.c +++ b/pd/src/s_path.c @@ -660,7 +660,6 @@ int sys_rcfile(void) /* parse the options */ - fclose(file); if (sys_verbose) { if (rcargc) @@ -681,8 +680,10 @@ int sys_rcfile(void) cleanup: /* prevent memleak */ + fclose(file); + for (i = 1; i < NUMARGS-1; i++) - if(rcargv[i])free(rcargv[i]); + if (rcargv[i]) free(rcargv[i]); return(retval); } diff --git a/pd/src/u_pdreceive.c b/pd/src/u_pdreceive.c index 279ce90cd..3fbb8f083 100644 --- a/pd/src/u_pdreceive.c +++ b/pd/src/u_pdreceive.c @@ -51,7 +51,7 @@ static void dopoll(void); int main(int argc, char **argv) { int portno; - struct sockaddr_in server; + struct sockaddr_in server = {0}; #ifdef MSW short version = MAKEWORD(2, 0); WSADATA nobby; diff --git a/pd/src/u_pdsend.c b/pd/src/u_pdsend.c index 05090420a..57390ca11 100644 --- a/pd/src/u_pdsend.c +++ b/pd/src/u_pdsend.c @@ -29,7 +29,7 @@ void x_closesocket(int fd); int main(int argc, char **argv) { int sockfd, portno, protocol; - struct sockaddr_in server; + struct sockaddr_in server = {0}; struct hostent *hp; char *hostname; #if 0 -- GitLab