Skip to content

Commit 4dc1a2d

Browse files
committed
xbm: avoid stack overflow (read) with large names #211
We use the name passed in to printf into a local stack buffer which is limited to 4000 bytes. So given a large enough value, lots of stack data is leaked. Rewrite the code to do simple memory copies with most of the strings to avoid that issue, and only use stack buffer for small numbers of constant size. This closes #211.
1 parent b083ec1 commit 4dc1a2d

File tree

1 file changed

+27
-7
lines changed

1 file changed

+27
-7
lines changed

Diff for: src/gd_xbm.c

+27-7
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ BGD_DECLARE(gdImagePtr) gdImageCreateFromXbm(FILE * fd)
180180
/* {{{ gdCtxPrintf */
181181
static void gdCtxPrintf(gdIOCtx * out, const char *format, ...)
182182
{
183-
char buf[4096];
183+
char buf[1024];
184184
int len;
185185
va_list args;
186186

@@ -191,6 +191,9 @@ static void gdCtxPrintf(gdIOCtx * out, const char *format, ...)
191191
}
192192
/* }}} */
193193

194+
/* The compiler will optimize strlen(constant) to a constant number. */
195+
#define gdCtxPuts(out, s) out->putBuf(out, s, strlen(s))
196+
194197
/* {{{ gdImageXbmCtx */
195198
BGD_DECLARE(void) gdImageXbmCtx(gdImagePtr image, char* file_name, int fg, gdIOCtx * out)
196199
{
@@ -215,9 +218,26 @@ BGD_DECLARE(void) gdImageXbmCtx(gdImagePtr image, char* file_name, int fg, gdIOC
215218
}
216219
}
217220

218-
gdCtxPrintf(out, "#define %s_width %d\n", name, gdImageSX(image));
219-
gdCtxPrintf(out, "#define %s_height %d\n", name, gdImageSY(image));
220-
gdCtxPrintf(out, "static unsigned char %s_bits[] = {\n ", name);
221+
/* Since "name" comes from the user, run it through a direct puts.
222+
* Trying to printf it into a local buffer means we'd need a large
223+
* or dynamic buffer to hold it all. */
224+
225+
/* #define <name>_width 1234 */
226+
gdCtxPuts(out, "#define ");
227+
gdCtxPuts(out, name);
228+
gdCtxPuts(out, "_width ");
229+
gdCtxPrintf(out, "%d\n", gdImageSX(image));
230+
231+
/* #define <name>_height 1234 */
232+
gdCtxPuts(out, "#define ");
233+
gdCtxPuts(out, name);
234+
gdCtxPuts(out, "_height ");
235+
gdCtxPrintf(out, "%d\n", gdImageSY(image));
236+
237+
/* static unsigned char <name>_bits[] = {\n */
238+
gdCtxPuts(out, "static unsigned char ");
239+
gdCtxPuts(out, name);
240+
gdCtxPuts(out, "_bits[] = {\n ");
221241

222242
free(name);
223243

@@ -234,9 +254,9 @@ BGD_DECLARE(void) gdImageXbmCtx(gdImagePtr image, char* file_name, int fg, gdIOC
234254
if ((b == 128) || (x == sx && y == sy)) {
235255
b = 1;
236256
if (p) {
237-
gdCtxPrintf(out, ", ");
257+
gdCtxPuts(out, ", ");
238258
if (!(p%12)) {
239-
gdCtxPrintf(out, "\n ");
259+
gdCtxPuts(out, "\n ");
240260
p = 12;
241261
}
242262
}
@@ -248,6 +268,6 @@ BGD_DECLARE(void) gdImageXbmCtx(gdImagePtr image, char* file_name, int fg, gdIOC
248268
}
249269
}
250270
}
251-
gdCtxPrintf(out, "};\n");
271+
gdCtxPuts(out, "};\n");
252272
}
253273
/* }}} */

0 commit comments

Comments
 (0)