From: Emden R. Gansner Date: Tue, 6 Jan 2015 20:59:38 +0000 (-0500) Subject: Various fixes to prevent buffer overflows, cleanup usage of std_args, fix logical... X-Git-Tag: TRAVIS_CI_BUILD_EXPERIMENTAL~129^2~5^2~4 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6fec15b58f1e3134c8fa7eb17137ef49330775bf;p=graphviz Various fixes to prevent buffer overflows, cleanup usage of std_args, fix logical problems, unfreed memory, and bad declarations. These were submitted by Joshua Rogers. --- diff --git a/cmd/lefty/common.c b/cmd/lefty/common.c index fbe41bc66..9a3796914 100644 --- a/cmd/lefty/common.c +++ b/cmd/lefty/common.c @@ -17,6 +17,8 @@ int warnflag; char *leftypath, *leftyoptions, *shellpath; +static int leftypathsz; +static int leftypathlen; jmp_buf exitljbuf; int idlerunmode; fd_set inputfds; @@ -57,6 +59,20 @@ static char *cmdp; static char *lpathp; +static void pathAppend(char* s, int addSep) +{ + int newlen = leftypathlen + strlen(s) + addSep; + + if (newlen >= leftypathsz) { + leftypathsz = newlen + 1024; + if (!(leftypath = realloc (leftypath, leftypathsz))) + panic1 (POS, "init", "pathp realloc failed"); + } + if (*s) strcat (leftypath, s); + if (addSep) strcat (leftypath, PATHSEPSTR); + leftypathlen = newlen; +} + int init (char *aout) { char *s1, *s2, c; int k; @@ -92,16 +108,15 @@ int init (char *aout) { if (!(s1 = strrchr (aout, PATHDEL))) s1 = aout; *s1 = 0; - if (!(leftypath = malloc (PATHINCR * PATHSIZE))) + leftypathsz = PATHINCR * PATHSIZE; + if (!(leftypath = malloc (leftypathsz))) panic1 (POS, "init", "leftypath malloc failed"); leftypath[0] = 0; if ((s1 = getenv ("LEFTYPATH"))) { - strcat (leftypath, s1); - strcat (leftypath, PATHSEPSTR); + pathAppend (s1, 1); } if (*aout) { - strcat (leftypath, aout); - strcat (leftypath, PATHSEPSTR); + pathAppend (aout, 1); } for (k = 0; k < 2; k++) { if (k == 0) @@ -111,20 +126,19 @@ int init (char *aout) { while (s1) { if ((s2 = strchr (s1, PATHSEP))) c = *s2, *s2 = 0; - strcat (leftypath, s1); - strcat (leftypath, PATHLEFTY); + pathAppend (s1, 0); + pathAppend (PATHLEFTY, 0); if (s2) { *s2 = c, s1 = s2 + 1; - strcat (leftypath, PATHSEPSTR); + pathAppend ("", 1); } else s1 = NULL; } if (leftypath[0]) - strcat (leftypath, PATHSEPSTR); + pathAppend ("", 1); } if (leftdatadir) { /* support a compile-time path as last resort */ - strcat (leftypath, leftdatadir); - strcat (leftypath, PATHSEPSTR); + pathAppend (leftdatadir, 1); } if (!(leftyoptions = getenv ("LEFTYOPTIONS"))) leftyoptions = ""; diff --git a/cmd/lefty/g.c b/cmd/lefty/g.c index 49c4193d7..bd3e6c382 100644 --- a/cmd/lefty/g.c +++ b/cmd/lefty/g.c @@ -207,7 +207,7 @@ static char *errmsg[] = { /* G_ERRNOSUCHCURSOR */ "no such cursor %s", /* G_ERRNOTACANVAS */ "widget %d is not a canvas", /* G_ERRNOTIMPLEMENTED */ "not implemented", - /* G_ERRNOTSUPPORTED */ "not supported" + /* G_ERRNOTSUPPORTED */ "not supported", /* G_ERRBADBITMAPID */ "bad bitmap id %d", /* G_ERRCANNOTCREATEBITMAP */ "cannot create bitmap", /* G_ERRNOBITMAP */ "no bitmap", diff --git a/cmd/tools/gvcolor.c b/cmd/tools/gvcolor.c index 830fdd8e3..a7993deb6 100644 --- a/cmd/tools/gvcolor.c +++ b/cmd/tools/gvcolor.c @@ -237,6 +237,7 @@ static void color(Agraph_t * g) sprintf(buf, "%f %f %f", h, s, b); agset(n, "color", buf); } + free (nlist); } static Agraph_t *gread(FILE * fp) diff --git a/lib/ast/pathcanon.c b/lib/ast/pathcanon.c index d647aaf64..e1746af1b 100644 --- a/lib/ast/pathcanon.c +++ b/lib/ast/pathcanon.c @@ -50,6 +50,7 @@ char *pathcanon(char *path, int flags) register int dots; char *phys; char *v; + char* e = path + PATH_MAX; int loop; int oerrno; #if defined(FS_3D) @@ -141,6 +142,10 @@ char *pathcanon(char *path, int flags) dots = pathgetlink(phys, buf, sizeof(buf)); *(t - 1) = c; if (dots > 0) { + if ((t + dots + 1) >= e) { /* make sure path fits in buf */ + strcpy(path, s); + return 0; + } loop++; strcpy(buf + dots, s - (*s != 0)); if (*buf == '/') diff --git a/lib/cgraph/agerror.c b/lib/cgraph/agerror.c index 64d9db3cb..718cefb40 100644 --- a/lib/cgraph/agerror.c +++ b/lib/cgraph/agerror.c @@ -135,16 +135,18 @@ static int agerr_va(agerrlevel_t level, const char *fmt, va_list args) if (level != AGPREV) aglast = ftell(agerrout); vfprintf(agerrout, fmt, args); - va_end(args); return 0; } int agerr(agerrlevel_t level, const char *fmt, ...) { va_list args; + int ret; va_start(args, fmt); - return agerr_va(level, fmt, args); + ret = agerr_va(level, fmt, args); + va_end(args); + return ret; } void agerrorf(const char *fmt, ...) @@ -153,6 +155,7 @@ void agerrorf(const char *fmt, ...) va_start(args, fmt); agerr_va(AGERR, fmt, args); + va_end(args); } void agwarningf(const char *fmt, ...) @@ -161,6 +164,7 @@ void agwarningf(const char *fmt, ...) va_start(args, fmt); agerr_va(AGWARN, fmt, args); + va_end(args); } int agerrors() { return agmaxerr; } diff --git a/lib/common/emit.c b/lib/common/emit.c index 6a6074687..cc2744aa0 100644 --- a/lib/common/emit.c +++ b/lib/common/emit.c @@ -187,7 +187,7 @@ layerPagePrefix (GVJ_t* job, agxbuf* xb) agxbput (xb, job->gvc->layerIDs[job->layerNum]); agxbputc (xb, '_'); } - if ((job->pagesArrayElem.x > 0) || (job->pagesArrayElem.x > 0)) { + if ((job->pagesArrayElem.x > 0) || (job->pagesArrayElem.y > 0)) { sprintf (buf, "page%d,%d_", job->pagesArrayElem.x, job->pagesArrayElem.y); agxbput (xb, buf); } @@ -2693,30 +2693,40 @@ static void nodeIntersect (GVJ_t * job, pointf p, { obj_state_t *obj = job->obj; char* url; +#if 0 char* tooltip; char* target; +#endif boolean explicit; if (explicit_iurl) url = iurl; else url = obj->url; if (explicit_itooltip) { +#if 0 tooltip = itooltip; +#endif explicit = TRUE; } else if (obj->explicit_tooltip) { +#if 0 tooltip = obj->tooltip; +#endif explicit = TRUE; } else { - explicit = FALSE; +#if 0 tooltip = itooltip; +#endif + explicit = FALSE; } +#if 0 if (explicit_itarget) target = itarget; else if (obj->explicit_edgetarget) target = obj->target; else target = itarget; +#endif if (url || explicit) { map_point(job, p); @@ -2921,7 +2931,7 @@ boxf xdotBB (Agraph_t* g) double fontsize = 0.0; char* fontname = NULL; pointf pts[2]; - pointf sz; + /* pointf sz; */ boxf bb0; boxf bb = GD_bb(g); xdot* xd = (xdot*)GD_drawing(g)->xdots; @@ -2969,7 +2979,7 @@ boxf xdotBB (Agraph_t* g) tf.size = fontsize; tf.flags = fontflags; op->span->font = dtinsert(gvc->textfont_dt, &tf); - sz = textspan_size (gvc, op->span); + textspan_size (gvc, op->span); bb0 = textBB (op->op.u.text.x, op->op.u.text.y, op->span); op->bb = bb0; expandBB (&bb, bb0.LL); @@ -3756,7 +3766,7 @@ static boolean is_style_delim(int c) static int style_token(char **s, agxbuf * xb) { char *p = *s; - int token, rc; + int token; char c; while (*p && (isspace(*p) || (*p == ','))) @@ -3772,7 +3782,7 @@ static int style_token(char **s, agxbuf * xb) default: token = SID; while (!is_style_delim(c = *p)) { - rc = agxbputc(xb, c); + agxbputc(xb, c); p++; } } diff --git a/lib/dotgen/rank.c b/lib/dotgen/rank.c index 39c08dac0..a3b20f025 100644 --- a/lib/dotgen/rank.c +++ b/lib/dotgen/rank.c @@ -1035,7 +1035,11 @@ static void break_cycles(graph_t * g) for (n = agfstnode(g); n; n = agnxtnode(g, n)) dfs(g, n); } - +/* setMinMax: + * This will only be called with the root graph or a cluster + * which are guarenteed to contain nodes. Thus, leader will be + * set. + */ static void setMinMax (graph_t* g, int doRoot) { int c, v; diff --git a/lib/gvc/gvdevice.c b/lib/gvc/gvdevice.c index ecc520251..fdd8c905a 100644 --- a/lib/gvc/gvdevice.c +++ b/lib/gvc/gvdevice.c @@ -393,7 +393,7 @@ void gvdevice_finalize(GVJ_t * job) void gvprintf(GVJ_t * job, const char *format, ...) { char buf[BUFSIZ]; - size_t len; + int len; va_list argp; char* bp = buf; diff --git a/lib/gvc/gvusershape.c b/lib/gvc/gvusershape.c index 7869a26a2..ebbac1989 100644 --- a/lib/gvc/gvusershape.c +++ b/lib/gvc/gvusershape.c @@ -643,17 +643,20 @@ static usershape_t *gvusershape_open (char *name) return NULL; us->name = agstrdup (0, name); - if (!gvusershape_file_access(us)) + if (!gvusershape_file_access(us)) { + free(us); return NULL; + } assert(us->f); switch(imagetype(us)) { case FT_NULL: - if (!(us->data = (void*)find_user_shape(us->name))) + if (!(us->data = (void*)find_user_shape(us->name))) { agerr(AGWARN, "\"%s\" was not found as a file or as a shape library member\n", us->name); free(us); return NULL; + } break; case FT_GIF: gif_size(us); diff --git a/lib/sfio/sfprints.c b/lib/sfio/sfprints.c index 0d994ab54..e9159de9d 100644 --- a/lib/sfio/sfprints.c +++ b/lib/sfio/sfprints.c @@ -42,8 +42,10 @@ va_dcl /* make a fake stream */ if (!f && !(f = sfnew(NIL(Sfio_t *), NIL(char *), (size_t) SF_UNBOUND, - -1, SF_WRITE | SF_STRING))) - return NIL(char *); + -1, SF_WRITE | SF_STRING))) { + va_end(args); + return NIL(char *); + } sfseek(f, (Sfoff_t) 0, 0); rv = sfvprintf(f, form, args); diff --git a/plugin/core/gvrender_core_pic.c b/plugin/core/gvrender_core_pic.c index 4ba1b0e76..68547831f 100644 --- a/plugin/core/gvrender_core_pic.c +++ b/plugin/core/gvrender_core_pic.c @@ -450,7 +450,7 @@ static void pic_textspan(GVJ_t * job, pointf p, textspan_t * span) gvprintf(job, ".ft %s\n", picfontname(span->font->name)); lastname = span->font->name; } - if ((sz = (int)span->font->size) < 1); + if ((sz = (int)span->font->size) < 1) sz = 1; if (sz != lastsize) { gvprintf(job, ".ps %d*\\n(SFu/%.0fu\n", sz, Fontscale); diff --git a/plugin/core/gvrender_core_pov.c b/plugin/core/gvrender_core_pov.c index 7b253e51e..13ab1e58d 100644 --- a/plugin/core/gvrender_core_pov.c +++ b/plugin/core/gvrender_core_pov.c @@ -376,7 +376,7 @@ char *el(GVJ_t* job, char *template, ...) return str; #elif defined(HAVE_VSNPRINTF) char buf[BUFSIZ]; - size_t len; + int len; char *str; va_list arglist;