Skip to content

Commit

Permalink
Security fixes: expect final changes for release 1.75.3.
Browse files Browse the repository at this point in the history 
* Fixed a debian security issue with fscanf() reading a string with
  possible buffer overflow.
* There were also a few similar situations with sscanf().
  • Loading branch information
@DanBloomberg
DanBloomberg committed Feb 15, 2018
1 parent 24bfc6c commit ee301cb
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 55 deletions.
3 changes: 2 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ www.leptonica.org


## Open Source Projects that use Leptonica ##
* [php](http://en.wikipedia.org/wiki/PHP) (scripting language for dynamic web pages)
* [tesseract](https://github.com/tesseract-ocr/tesseract/) (optical character recognition)
* [OpenCV](https://github.com/opencv/opencv) (computer vision library)

This comment has been minimized.

Sign in to view

Copy link
@egorpugin

egorpugin Feb 17, 2018

Collaborator

By the way, where is leptonica used in opencv? In contrib.text or somewhere else?

This comment has been minimized.

Sign in to view

Copy link
@DanBloomberg

DanBloomberg via email Feb 18, 2018

Author Owner
* [jbig2enc](http://www.imperialviolet.org/jbig2.html) (encodes multipage binary image documents with jbig2 compression)
* [php](http://en.wikipedia.org/wiki/PHP) (scripting language for dynamic web pages)

## Major contributors to Leptonica ##
* Tom Powers: Tom has supported Leptonica on Windows for many years. He has made many contributions to code quality and documentation, including the beautiful "unofficial documentation" on the web site. Without his effort, Leptonica would not run today on Windows.
Expand Down
14 changes: 7 additions & 7 deletions prog/xtractprotos.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
#include <string.h>
#include "allheaders.h"

static const l_int32 L_BUF_SIZE = 512;
static const l_int32 L_BUFSIZE = 512; /* hardcoded below in sscanf() */
static const char *version = "1.5";


Expand All @@ -96,7 +96,7 @@ int main(int argc,
{
char *filein, *str, *tempfile, *prestring, *outprotos, *protostr;
const char *spacestr = " ";
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
l_uint8 *allheaders;
l_int32 i, maxindex, in_line, nflags, protos_added, firstfile, len, ret;
size_t nbytes;
Expand Down Expand Up @@ -125,12 +125,12 @@ static char mainName[] = "xtractprotos";
if (argv[i][0] == '-') {
if (!strncmp(argv[i], "-prestring", 10)) {
nflags++;
ret = sscanf(argv[i] + 1, "prestring=%s", buf);
ret = sscanf(argv[i] + 1, "prestring=%490s", buf);
if (ret != 1) {
fprintf(stderr, "parse failure for prestring\n");
return 1;
}
if ((len = strlen(buf)) > L_BUF_SIZE - 3) {
if ((len = strlen(buf)) > L_BUFSIZE - 3) {
L_WARNING("prestring too large; omitting!\n", mainName);
} else {
buf[len] = ' ';
Expand All @@ -139,7 +139,7 @@ static char mainName[] = "xtractprotos";
}
} else if (!strncmp(argv[i], "-protos", 7)) {
nflags++;
ret = sscanf(argv[i] + 1, "protos=%s", buf);
ret = sscanf(argv[i] + 1, "protos=%490s", buf);
if (ret != 1) {
fprintf(stderr, "parse failure for protos\n");
return 1;
Expand All @@ -165,7 +165,7 @@ static char mainName[] = "xtractprotos";
/* First the extern C head */
sa = sarrayCreate(0);
sarrayAddString(sa, (char *)"/*", L_COPY);
snprintf(buf, L_BUF_SIZE,
snprintf(buf, L_BUFSIZE,
" * These prototypes were autogen'd by xtractprotos, v. %s",
version);
sarrayAddString(sa, buf, L_COPY);
Expand All @@ -190,7 +190,7 @@ static char mainName[] = "xtractprotos";
len = strlen(filein);
if (filein[len - 1] == 'h') /* skip .h files */
continue;
snprintf(buf, L_BUF_SIZE, "cpp -ansi -DNO_PROTOS %s %s",
snprintf(buf, L_BUFSIZE, "cpp -ansi -DNO_PROTOS %s %s",
filein, tempfile);
ret = system(buf); /* cpp */
if (ret) {
Expand Down
72 changes: 36 additions & 36 deletions src/gplot.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@
* l_int32 gplotWrite()
*
*
* Utility for programmatic plotting using gnuplot 7.3.2 or later
* Utility for programmatic plotting using gnuplot 4.6 or later
* Enabled:
* ~ output to png (color), ps (mono), x11 (color), latex (mono)
* ~ output to png (color), ps and eps (mono), latex (mono)
* ~ optional title for graph
* ~ optional x and y axis labels
* ~ multiple plots on one frame
Expand Down Expand Up @@ -100,7 +100,7 @@
#include <string.h>
#include "allheaders.h"

static const l_int32 L_BUF_SIZE = 512;
static const l_int32 L_BUFSIZE = 512; /* hardcoded below in fscanf */

const char *gplotstylenames[] = {"with lines",
"with points",
Expand Down Expand Up @@ -142,7 +142,7 @@ gplotCreate(const char *rootname,
const char *ylabel)
{
char *newroot;
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
l_int32 badchar;
GPLOT *gplot;

Expand All @@ -169,16 +169,16 @@ GPLOT *gplot;
newroot = genPathname(rootname, NULL);
gplot->rootname = newroot;
gplot->outformat = outformat;
snprintf(buf, L_BUF_SIZE, "%s.cmd", rootname);
snprintf(buf, L_BUFSIZE, "%s.cmd", rootname);
gplot->cmdname = stringNew(buf);
if (outformat == GPLOT_PNG)
snprintf(buf, L_BUF_SIZE, "%s.png", newroot);
snprintf(buf, L_BUFSIZE, "%s.png", newroot);

This comment has been minimized.

Sign in to view

Copy link
@setharnold

setharnold Feb 17, 2018

If newroot is 508 characters or more, the results of buf will not contain (some or all of) the .png contents. I always like to see the return value of snprintf() checked not just for less than zero (obvious failures) but also greater than the buf size, a kind of failure that may be every bit as catastrophic as a buffer overflow. None of these cases look that severe. But I'd like to encourage the habit of checking the error return value from snprintf(). Thanks.

This comment has been minimized.

Sign in to view

Copy link
@DanBloomberg

DanBloomberg Feb 17, 2018

Author Owner

Thank you for the suggestion -- it makes sense.

I'll check this out next week and see what the failure modes with big strings look like.
else if (outformat == GPLOT_PS)
snprintf(buf, L_BUF_SIZE, "%s.ps", newroot);
snprintf(buf, L_BUFSIZE, "%s.ps", newroot);
else if (outformat == GPLOT_EPS)
snprintf(buf, L_BUF_SIZE, "%s.eps", newroot);
snprintf(buf, L_BUFSIZE, "%s.eps", newroot);
else if (outformat == GPLOT_LATEX)
snprintf(buf, L_BUF_SIZE, "%s.tex", newroot);
snprintf(buf, L_BUFSIZE, "%s.tex", newroot);
gplot->outname = stringNew(buf);
if (title) gplot->title = stringNew(title);
if (xlabel) gplot->xlabel = stringNew(xlabel);
Expand Down Expand Up @@ -266,7 +266,7 @@ gplotAddPlot(GPLOT *gplot,
l_int32 plotstyle,
const char *plottitle)
{
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
char emptystring[] = "";
char *datastr, *title;
l_int32 n, i;
Expand Down Expand Up @@ -303,7 +303,7 @@ SARRAY *sa;

/* Generate and save data filename */
gplot->nplots++;
snprintf(buf, L_BUF_SIZE, "%s.data.%d", gplot->rootname, gplot->nplots);
snprintf(buf, L_BUFSIZE, "%s.data.%d", gplot->rootname, gplot->nplots);
sarrayAddString(gplot->datanames, buf, L_COPY);

/* Generate data and save as a string */
Expand All @@ -314,7 +314,7 @@ SARRAY *sa;
else
valx = startx + i * delx;
numaGetFValue(nay, i, &valy);
snprintf(buf, L_BUF_SIZE, "%f %f\n", valx, valy);
snprintf(buf, L_BUFSIZE, "%f %f\n", valx, valy);
sarrayAddString(sa, buf, L_COPY);
}
datastr = sarrayToString(sa, 0);
Expand Down Expand Up @@ -378,7 +378,7 @@ gplotSetScaling(GPLOT *gplot,
l_int32
gplotMakeOutput(GPLOT *gplot)
{
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
char *cmdname;
l_int32 ignore;

Expand All @@ -392,9 +392,9 @@ l_int32 ignore;
cmdname = genPathname(gplot->cmdname, NULL);

#ifndef _WIN32
snprintf(buf, L_BUF_SIZE, "gnuplot %s", cmdname);
snprintf(buf, L_BUFSIZE, "gnuplot %s", cmdname);
#else
snprintf(buf, L_BUF_SIZE, "wgnuplot %s", cmdname);
snprintf(buf, L_BUFSIZE, "wgnuplot %s", cmdname);
#endif /* _WIN32 */

#ifndef OS_IOS /* iOS 11 does not support system() */
Expand All @@ -415,7 +415,7 @@ l_int32 ignore;
l_int32
gplotGenCommandFile(GPLOT *gplot)
{
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
char *cmdstr, *plottitle, *dataname;
l_int32 i, plotstyle, nplots;
FILE *fp;
Expand All @@ -430,43 +430,43 @@ FILE *fp;

/* Generate command data instructions */
if (gplot->title) { /* set title */
snprintf(buf, L_BUF_SIZE, "set title '%s'", gplot->title);
snprintf(buf, L_BUFSIZE, "set title '%s'", gplot->title);
sarrayAddString(gplot->cmddata, buf, L_COPY);
}
if (gplot->xlabel) { /* set xlabel */
snprintf(buf, L_BUF_SIZE, "set xlabel '%s'", gplot->xlabel);
snprintf(buf, L_BUFSIZE, "set xlabel '%s'", gplot->xlabel);
sarrayAddString(gplot->cmddata, buf, L_COPY);
}
if (gplot->ylabel) { /* set ylabel */
snprintf(buf, L_BUF_SIZE, "set ylabel '%s'", gplot->ylabel);
snprintf(buf, L_BUFSIZE, "set ylabel '%s'", gplot->ylabel);
sarrayAddString(gplot->cmddata, buf, L_COPY);
}

/* Set terminal type and output */
if (gplot->outformat == GPLOT_PNG) {
snprintf(buf, L_BUF_SIZE, "set terminal png; set output '%s'",
snprintf(buf, L_BUFSIZE, "set terminal png; set output '%s'",
gplot->outname);
} else if (gplot->outformat == GPLOT_PS) {
snprintf(buf, L_BUF_SIZE, "set terminal postscript; set output '%s'",
snprintf(buf, L_BUFSIZE, "set terminal postscript; set output '%s'",
gplot->outname);
} else if (gplot->outformat == GPLOT_EPS) {
snprintf(buf, L_BUF_SIZE,
snprintf(buf, L_BUFSIZE,
"set terminal postscript eps; set output '%s'",
gplot->outname);
} else if (gplot->outformat == GPLOT_LATEX) {
snprintf(buf, L_BUF_SIZE, "set terminal latex; set output '%s'",
snprintf(buf, L_BUFSIZE, "set terminal latex; set output '%s'",
gplot->outname);
}
sarrayAddString(gplot->cmddata, buf, L_COPY);

if (gplot->scaling == GPLOT_LOG_SCALE_X ||
gplot->scaling == GPLOT_LOG_SCALE_X_Y) {
snprintf(buf, L_BUF_SIZE, "set logscale x");
snprintf(buf, L_BUFSIZE, "set logscale x");
sarrayAddString(gplot->cmddata, buf, L_COPY);
}
if (gplot->scaling == GPLOT_LOG_SCALE_Y ||
gplot->scaling == GPLOT_LOG_SCALE_X_Y) {
snprintf(buf, L_BUF_SIZE, "set logscale y");
snprintf(buf, L_BUFSIZE, "set logscale y");
sarrayAddString(gplot->cmddata, buf, L_COPY);
}

Expand All @@ -476,17 +476,17 @@ FILE *fp;
dataname = sarrayGetString(gplot->datanames, i, L_NOCOPY);
numaGetIValue(gplot->plotstyles, i, &plotstyle);
if (nplots == 1) {
snprintf(buf, L_BUF_SIZE, "plot '%s' title '%s' %s",
snprintf(buf, L_BUFSIZE, "plot '%s' title '%s' %s",
dataname, plottitle, gplotstylenames[plotstyle]);
} else {
if (i == 0)
snprintf(buf, L_BUF_SIZE, "plot '%s' title '%s' %s, \\",
snprintf(buf, L_BUFSIZE, "plot '%s' title '%s' %s, \\",
dataname, plottitle, gplotstylenames[plotstyle]);
else if (i < nplots - 1)
snprintf(buf, L_BUF_SIZE, " '%s' title '%s' %s, \\",
snprintf(buf, L_BUFSIZE, " '%s' title '%s' %s, \\",
dataname, plottitle, gplotstylenames[plotstyle]);
else
snprintf(buf, L_BUF_SIZE, " '%s' title '%s' %s",
snprintf(buf, L_BUFSIZE, " '%s' title '%s' %s",
dataname, plottitle, gplotstylenames[plotstyle]);
}
sarrayAddString(gplot->cmddata, buf, L_COPY);
Expand Down Expand Up @@ -820,7 +820,7 @@ NUMA *nay;
GPLOT *
gplotRead(const char *filename)
{
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
char *rootname, *title, *xlabel, *ylabel, *ignores;
l_int32 outformat, ret, version, ignore;
FILE *fp;
Expand All @@ -844,16 +844,16 @@ GPLOT *gplot;
return (GPLOT *)ERROR_PTR("invalid gplot version", procName, NULL);
}

ignore = fscanf(fp, "Rootname: %s\n", buf);
ignore = fscanf(fp, "Rootname: %511s\n", buf); /* L_BUFSIZE - 1 */
rootname = stringNew(buf);
ignore = fscanf(fp, "Output format: %d\n", &outformat);
ignores = fgets(buf, L_BUF_SIZE, fp); /* Title: ... */
ignores = fgets(buf, L_BUFSIZE, fp); /* Title: ... */
title = stringNew(buf + 7);
title[strlen(title) - 1] = '\0';
ignores = fgets(buf, L_BUF_SIZE, fp); /* X axis label: ... */
ignores = fgets(buf, L_BUFSIZE, fp); /* X axis label: ... */
xlabel = stringNew(buf + 14);
xlabel[strlen(xlabel) - 1] = '\0';
ignores = fgets(buf, L_BUF_SIZE, fp); /* Y axis label: ... */
ignores = fgets(buf, L_BUFSIZE, fp); /* Y axis label: ... */
ylabel = stringNew(buf + 14);
ylabel[strlen(ylabel) - 1] = '\0';

Expand All @@ -872,7 +872,7 @@ GPLOT *gplot;
sarrayDestroy(&gplot->plottitles);
numaDestroy(&gplot->plotstyles);

ignore = fscanf(fp, "Commandfile name: %s\n", buf);
ignore = fscanf(fp, "Commandfile name: %511s\n", buf); /* L_BUFSIZE - 1 */
stringReplace(&gplot->cmdname, buf);
ignore = fscanf(fp, "\nCommandfile data:");
gplot->cmddata = sarrayReadStream(fp);
Expand All @@ -886,7 +886,7 @@ GPLOT *gplot;
gplot->plotstyles = numaReadStream(fp);

ignore = fscanf(fp, "Number of plots: %d\n", &gplot->nplots);
ignore = fscanf(fp, "Output file name: %s\n", buf);
ignore = fscanf(fp, "Output file name: %511s\n", buf);
stringReplace(&gplot->outname, buf);
ignore = fscanf(fp, "Axis scaling: %d\n", &gplot->scaling);

Expand Down
4 changes: 2 additions & 2 deletions src/ptabasic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ PTA *pta;
PTA *
ptaReadStream(FILE *fp)
{
char typestr[128];
char typestr[128]; /* hardcoded below in fscanf */
l_int32 i, n, ix, iy, type, version;
l_float32 x, y;
PTA *pta;
Expand All @@ -703,7 +703,7 @@ PTA *pta;
return (PTA *)ERROR_PTR("not a pta file", procName, NULL);
if (version != PTA_VERSION_NUMBER)
return (PTA *)ERROR_PTR("invalid pta version", procName, NULL);
if (fscanf(fp, " Number of pts = %d; format = %s\n", &n, typestr) != 2)
if (fscanf(fp, " Number of pts = %d; format = %127s\n", &n, typestr) != 2)
return (PTA *)ERROR_PTR("not a pta file", procName, NULL);
if (!strcmp(typestr, "float"))
type = 0;
Expand Down
18 changes: 9 additions & 9 deletions src/sel1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
#include <string.h>
#include "allheaders.h"

static const l_int32 L_BUF_SIZE = 256;
static const l_int32 L_BUFSIZE = 256; /* hardcoded below in sscanf */
static const l_int32 INITIAL_PTR_ARRAYSIZE = 50; /* n'import quoi */
static const l_int32 MANY_SELS = 1000;

Expand Down Expand Up @@ -978,7 +978,7 @@ selaGetCombName(SELA *sela,
l_int32 direction)
{
char *selname;
char combname[L_BUF_SIZE];
char combname[L_BUFSIZE];
l_int32 i, nsels, sx, sy, found;
SEL *sel;

Expand All @@ -991,9 +991,9 @@ SEL *sel;

/* Derive the comb name we're looking for */
if (direction == L_HORIZ)
snprintf(combname, L_BUF_SIZE, "sel_comb_%dh", size);
snprintf(combname, L_BUFSIZE, "sel_comb_%dh", size);
else /* direction == L_VERT */
snprintf(combname, L_BUF_SIZE, "sel_comb_%dv", size);
snprintf(combname, L_BUFSIZE, "sel_comb_%dv", size);

found = FALSE;
nsels = selaGetCount(sela);
Expand Down Expand Up @@ -1041,7 +1041,7 @@ static void
selaComputeCompositeParameters(const char *fileout)
{
char *str, *nameh1, *nameh2, *namev1, *namev2;
char buf[L_BUF_SIZE];
char buf[L_BUFSIZE];
l_int32 size, size1, size2, len;
SARRAY *sa;
SELA *selabasic, *selacomb;
Expand All @@ -1060,7 +1060,7 @@ SELA *selabasic, *selacomb;
nameh2 = stringNew("");
namev2 = stringNew("");
}
snprintf(buf, L_BUF_SIZE,
snprintf(buf, L_BUFSIZE,
" { %d, %d, %d, \"%s\", \"%s\", \"%s\", \"%s\" },",
size, size1, size2, nameh1, nameh2, namev1, namev2);
sarrayAddString(sa, buf, L_COPY);
Expand Down Expand Up @@ -1409,7 +1409,7 @@ SEL *
selReadStream(FILE *fp)
{
char *selname;
char linebuf[L_BUF_SIZE];
char linebuf[L_BUFSIZE];
l_int32 sy, sx, cy, cx, i, j, version, ignore;
SEL *sel;

Expand All @@ -1423,10 +1423,10 @@ SEL *sel;
if (version != SEL_VERSION_NUMBER)
return (SEL *)ERROR_PTR("invalid sel version", procName, NULL);

if (fgets(linebuf, L_BUF_SIZE, fp) == NULL)
if (fgets(linebuf, L_BUFSIZE, fp) == NULL)
return (SEL *)ERROR_PTR("error reading into linebuf", procName, NULL);
selname = stringNew(linebuf);
sscanf(linebuf, " ------ %s ------", selname);
sscanf(linebuf, " ------ %200s ------", selname);

if (fscanf(fp, " sy = %d, sx = %d, cy = %d, cx = %d\n",
&sy, &sx, &cy, &cx) != 4) {
Expand Down

0 comments on commit ee301cb

Please sign in to comment.