summaryrefslogtreecommitdiff
path: root/src/CrDatFrI.c
diff options
context:
space:
mode:
Diffstat (limited to 'src/CrDatFrI.c')
-rw-r--r--src/CrDatFrI.c94
1 files changed, 77 insertions, 17 deletions
diff --git a/src/CrDatFrI.c b/src/CrDatFrI.c
index 55f8b06..37e7f8c 100644
--- a/src/CrDatFrI.c
+++ b/src/CrDatFrI.c
@@ -33,13 +33,16 @@
\*****************************************************************************/
/* $XFree86$ */
+/* October 2004, source code review by Thomas Biege <thomas@suse.de> */
+
#include "XpmI.h"
LFUNC(CreateColors, int, (char **dataptr, unsigned int *data_size,
XpmColor *colors, unsigned int ncolors,
unsigned int cpp));
-LFUNC(CreatePixels, void, (char **dataptr, unsigned int width,
+LFUNC(CreatePixels, void, (char **dataptr, unsigned int data_size,
+ unsigned int width,
unsigned int height, unsigned int cpp,
unsigned int *pixels, XpmColor *colors));
@@ -47,7 +50,8 @@ LFUNC(CountExtensions, void, (XpmExtension *ext, unsigned int num,
unsigned int *ext_size,
unsigned int *ext_nlines));
-LFUNC(CreateExtensions, void, (char **dataptr, unsigned int offset,
+LFUNC(CreateExtensions, void, (char **dataptr, unsigned int data_size,
+ unsigned int offset,
XpmExtension *ext, unsigned int num,
unsigned int ext_nlines));
@@ -88,10 +92,11 @@ XpmCreateDataFromImage(display, data_return, image, shapeimage, attributes)
#undef RETURN
#define RETURN(status) \
+do \
{ \
ErrorStatus = status; \
goto exit; \
-}
+} while(0)
int
XpmCreateDataFromXpmImage(data_return, image, info)
@@ -122,9 +127,17 @@ XpmCreateDataFromXpmImage(data_return, image, info)
* alloc a temporary array of char pointer for the header section which
* is the hints line + the color table lines
*/
- header_nlines = 1 + image->ncolors;
+ header_nlines = 1 + image->ncolors; /* this may wrap and/or become 0 */
+
+ /* 2nd check superfluous if we do not need header_nlines any further */
+ if(header_nlines <= image->ncolors ||
+ header_nlines >= UINT_MAX / sizeof(char *))
+ return(XpmNoMemory);
+
header_size = sizeof(char *) * header_nlines;
- header = (char **) XpmCalloc(header_size, sizeof(char *));
+ if (header_size >= UINT_MAX / sizeof(char *))
+ return (XpmNoMemory);
+ header = (char **) XpmCalloc(header_size, sizeof(char *)); /* can we trust image->ncolors */
if (!header)
return (XpmNoMemory);
@@ -168,8 +181,22 @@ XpmCreateDataFromXpmImage(data_return, image, info)
/* now we know the size needed, alloc the data and copy the header lines */
offset = image->width * image->cpp + 1;
- data_size = header_size + (image->height + ext_nlines) * sizeof(char *)
- + image->height * offset + ext_size;
+
+ if(offset <= image->width || offset <= image->cpp)
+ RETURN(XpmNoMemory);
+
+ if( (image->height + ext_nlines) >= UINT_MAX / sizeof(char *))
+ RETURN(XpmNoMemory);
+ data_size = (image->height + ext_nlines) * sizeof(char *);
+
+ if (image->height > UINT_MAX / offset ||
+ image->height * offset > UINT_MAX - data_size)
+ RETURN(XpmNoMemory);
+ data_size += image->height * offset;
+
+ if( (header_size + ext_size) >= (UINT_MAX - data_size) )
+ RETURN(XpmNoMemory);
+ data_size += header_size + ext_size;
data = (char **) XpmMalloc(data_size);
if (!data)
@@ -177,8 +204,10 @@ XpmCreateDataFromXpmImage(data_return, image, info)
data_nlines = header_nlines + image->height + ext_nlines;
*data = (char *) (data + data_nlines);
+
+ /* can header have less elements then n suggests? */
n = image->ncolors;
- for (l = 0, sptr = data, sptr2 = header; l <= n; l++, sptr++, sptr2++) {
+ for (l = 0, sptr = data, sptr2 = header; l <= n && sptr && sptr2; l++, sptr++, sptr2++) {
strcpy(*sptr, *sptr2);
*(sptr + 1) = *sptr + strlen(*sptr2) + 1;
}
@@ -187,12 +216,13 @@ XpmCreateDataFromXpmImage(data_return, image, info)
data[header_nlines] = (char *) data + header_size
+ (image->height + ext_nlines) * sizeof(char *);
- CreatePixels(data + header_nlines, image->width, image->height,
+ CreatePixels(data + header_nlines, data_size-header_nlines, image->width, image->height,
image->cpp, image->data, image->colorTable);
/* print extensions */
if (extensions)
- CreateExtensions(data + header_nlines + image->height - 1, offset,
+ CreateExtensions(data + header_nlines + image->height - 1,
+ data_size - header_nlines - image->height + 1, offset,
info->extensions, info->nextensions,
ext_nlines);
@@ -223,23 +253,34 @@ CreateColors(dataptr, data_size, colors, ncolors, cpp)
char *s, *s2;
char **defaults;
+ /* can ncolors be trusted here? */
for (a = 0; a < ncolors; a++, colors++, dataptr++) {
defaults = (char **) colors;
+ if(sizeof(buf) <= cpp)
+ return(XpmNoMemory);
strncpy(buf, *defaults++, cpp);
s = buf + cpp;
+ if(sizeof(buf) <= (s-buf))
+ return XpmNoMemory;
+
for (key = 1; key <= NKEYS; key++, defaults++) {
if ((s2 = *defaults)) {
#ifndef VOID_SPRINTF
s +=
#endif
- sprintf(s, "\t%s %s", xpmColorKeys[key - 1], s2);
+ /* assume C99 compliance */
+ snprintf(s, sizeof(buf)-(s-buf), "\t%s %s", xpmColorKeys[key - 1], s2);
#ifdef VOID_SPRINTF
s += strlen(s);
#endif
+ /* does s point out-of-bounds? */
+ if(sizeof(buf) < (s-buf))
+ return XpmNoMemory;
}
}
+ /* what about using strdup()? */
l = s - buf + 1;
s = (char *) XpmMalloc(l);
if (!s)
@@ -251,8 +292,9 @@ CreateColors(dataptr, data_size, colors, ncolors, cpp)
}
static void
-CreatePixels(dataptr, width, height, cpp, pixels, colors)
+CreatePixels(dataptr, data_size, width, height, cpp, pixels, colors)
char **dataptr;
+ unsigned int data_size;
unsigned int width;
unsigned int height;
unsigned int cpp;
@@ -262,21 +304,38 @@ CreatePixels(dataptr, width, height, cpp, pixels, colors)
char *s;
unsigned int x, y, h, offset;
+ if(height <= 1)
+ return;
+
h = height - 1;
+
offset = width * cpp + 1;
+
+ if(offset <= width || offset <= cpp)
+ return;
+
+ /* why trust h? */
for (y = 0; y < h; y++, dataptr++) {
s = *dataptr;
+ /* why trust width? */
for (x = 0; x < width; x++, pixels++) {
- strncpy(s, colors[*pixels].string, cpp);
+ if(cpp > (data_size - (s - *dataptr)))
+ return;
+ strncpy(s, colors[*pixels].string, cpp); /* why trust pixel? */
s += cpp;
}
*s = '\0';
+ if(offset > data_size)
+ return;
*(dataptr + 1) = *dataptr + offset;
}
/* duplicate some code to avoid a test in the loop */
s = *dataptr;
+ /* why trust width? */
for (x = 0; x < width; x++, pixels++) {
- strncpy(s, colors[*pixels].string, cpp);
+ if(cpp > data_size - (s - *dataptr))
+ return;
+ strncpy(s, colors[*pixels].string, cpp); /* why should we trust *pixel? */
s += cpp;
}
*s = '\0';
@@ -309,8 +368,9 @@ CountExtensions(ext, num, ext_size, ext_nlines)
}
static void
-CreateExtensions(dataptr, offset, ext, num, ext_nlines)
+CreateExtensions(dataptr, data_size, offset, ext, num, ext_nlines)
char **dataptr;
+ unsigned int data_size;
unsigned int offset;
XpmExtension *ext;
unsigned int num;
@@ -323,12 +383,12 @@ CreateExtensions(dataptr, offset, ext, num, ext_nlines)
dataptr++;
a = 0;
for (x = 0; x < num; x++, ext++) {
- sprintf(*dataptr, "XPMEXT %s", ext->name);
+ snprintf(*dataptr, data_size, "XPMEXT %s", ext->name);
a++;
if (a < ext_nlines)
*(dataptr + 1) = *dataptr + strlen(ext->name) + 8;
dataptr++;
- b = ext->nlines;
+ b = ext->nlines; /* can we trust these values? */
for (y = 0, line = ext->lines; y < b; y++, line++) {
strcpy(*dataptr, *line);
a++;
generated by cgit v1.2.3 (git 2.39.1) at 2024-04-27 23:39:43 +0000