Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various checks on malloc #1781

Merged
merged 16 commits into from
Jun 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions _imaging.c
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,20 @@ getbands(const char* mode)
#define TYPE_DOUBLE (0x400|sizeof(double))

static void*
getlist(PyObject* arg, int* length, const char* wrong_length, int type)
{
int i, n, itemp;
getlist(PyObject* arg, Py_ssize_t* length, const char* wrong_length, int type)
{
/* - allocates and returns a c array of the items in the
python sequence arg.
- the size of the returned array is in length
- all of the arg items must be numeric items of the type
specified in type
- sequence length is checked against the length parameter IF
an error parameter is passed in wrong_length
- caller is responsible for freeing the memory
*/

Py_ssize_t i, n;
int itemp;
double dtemp;
void* list;
PyObject* seq;
Expand All @@ -381,7 +392,9 @@ getlist(PyObject* arg, int* length, const char* wrong_length, int type)
return NULL;
}

list = malloc(n * (type & 0xff));
/* malloc check ok, type & ff is just a sizeof(something)
calloc checks for overflow */
list = calloc(n, type & 0xff);
if (!list)
return PyErr_NoMemory();

Expand Down Expand Up @@ -845,7 +858,7 @@ static PyObject*
_filter(ImagingObject* self, PyObject* args)
{
PyObject* imOut;
int kernelsize;
Py_ssize_t kernelsize;
FLOAT32* kerneldata;

int xsize, ysize;
Expand All @@ -859,7 +872,7 @@ _filter(ImagingObject* self, PyObject* args)
kerneldata = getlist(kernel, &kernelsize, NULL, TYPE_FLOAT32);
if (!kerneldata)
return NULL;
if (kernelsize != xsize * ysize) {
if (kernelsize != (Py_ssize_t) xsize * (Py_ssize_t) ysize) {
free(kerneldata);
return ImagingError_ValueError("bad kernel size");
}
Expand Down Expand Up @@ -1148,8 +1161,8 @@ _point(ImagingObject* self, PyObject* args)
{
static const char* wrong_number = "wrong number of lut entries";

int n, i;
int bands;
Py_ssize_t n;
int i, bands;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should i also be Py_ssize_t?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, n is set to constant values well within the range of int in different locations in this function, and i counts up to that. The reason that n is a Py_ssize_t is that it's address is passed as an argument to getlist, which needs it to be a Py_ssize_t to match the result type of the python get sequence length function.

Imaging im;

PyObject* list;
Expand Down Expand Up @@ -1664,7 +1677,7 @@ _transform2(ImagingObject* self, PyObject* args)

Imaging imIn;
Imaging imOut;
int n;
Py_ssize_t n;
double *a;

ImagingObject* imagep;
Expand Down Expand Up @@ -1920,6 +1933,7 @@ _getprojection(ImagingObject* self, PyObject* args)
unsigned char* yprofile;
PyObject* result;

/* malloc check ok */
xprofile = malloc(self->image->xsize);
yprofile = malloc(self->image->ysize);

Expand Down Expand Up @@ -2366,7 +2380,7 @@ _draw_dealloc(ImagingDrawObject* self)
PyObject_Del(self);
}

extern int PyPath_Flatten(PyObject* data, double **xy);
extern Py_ssize_t PyPath_Flatten(PyObject* data, double **xy);

static PyObject*
_draw_ink(ImagingDrawObject* self, PyObject* args)
Expand All @@ -2387,7 +2401,7 @@ static PyObject*
_draw_arc(ImagingDrawObject* self, PyObject* args)
{
double* xy;
int n;
Py_ssize_t n;

PyObject* data;
int ink;
Expand Down Expand Up @@ -2423,7 +2437,7 @@ static PyObject*
_draw_bitmap(ImagingDrawObject* self, PyObject* args)
{
double *xy;
int n;
Py_ssize_t n;

PyObject *data;
ImagingObject* bitmap;
Expand Down Expand Up @@ -2459,7 +2473,7 @@ static PyObject*
_draw_chord(ImagingDrawObject* self, PyObject* args)
{
double* xy;
int n;
Py_ssize_t n;

PyObject* data;
int ink, fill;
Expand Down Expand Up @@ -2495,7 +2509,7 @@ static PyObject*
_draw_ellipse(ImagingDrawObject* self, PyObject* args)
{
double* xy;
int n;
Py_ssize_t n;

PyObject* data;
int ink;
Expand Down Expand Up @@ -2546,7 +2560,7 @@ static PyObject*
_draw_lines(ImagingDrawObject* self, PyObject* args)
{
double *xy;
int i, n;
Py_ssize_t i, n;

PyObject *data;
int ink;
Expand Down Expand Up @@ -2614,7 +2628,7 @@ static PyObject*
_draw_points(ImagingDrawObject* self, PyObject* args)
{
double *xy;
int i, n;
Py_ssize_t i, n;

PyObject *data;
int ink;
Expand Down Expand Up @@ -2676,7 +2690,7 @@ static PyObject*
_draw_pieslice(ImagingDrawObject* self, PyObject* args)
{
double* xy;
int n;
Py_ssize_t n;

PyObject* data;
int ink, fill;
Expand Down Expand Up @@ -2712,7 +2726,7 @@ _draw_polygon(ImagingDrawObject* self, PyObject* args)
{
double *xy;
int *ixy;
int n, i;
Py_ssize_t n, i;

PyObject* data;
int ink;
Expand All @@ -2731,7 +2745,7 @@ _draw_polygon(ImagingDrawObject* self, PyObject* args)
}

/* Copy list of vertices to array */
ixy = (int*) malloc(n * 2 * sizeof(int));
ixy = (int*) calloc(n, 2 * sizeof(int));

for (i = 0; i < n; i++) {
ixy[i+i] = (int) xy[i+i];
Expand All @@ -2756,7 +2770,7 @@ static PyObject*
_draw_rectangle(ImagingDrawObject* self, PyObject* args)
{
double* xy;
int n;
Py_ssize_t n;

PyObject* data;
int ink;
Expand Down
7 changes: 6 additions & 1 deletion decode.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,13 @@ _setimage(ImagingDecoderObject* decoder, PyObject* args)

/* Allocate memory buffer (if bits field is set) */
if (state->bits > 0) {
if (!state->bytes)
if (!state->bytes) {
if (state->xsize > ((INT_MAX / state->bits)-7)){
return PyErr_NoMemory();
}
state->bytes = (state->bits * state->xsize+7)/8;
}
/* malloc check ok, oveflow checked above */
state->buffer = (UINT8*) malloc(state->bytes);
if (!state->buffer)
return PyErr_NoMemory();
Expand Down
29 changes: 20 additions & 9 deletions encode.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ _encode_to_file(ImagingEncoderObject* encoder, PyObject* args)
return NULL;

/* Allocate an encoder buffer */
/* malloc check ok, either constant int, or checked by PyArg_ParseTuple */
buf = (UINT8*) malloc(bufsize);
if (!buf)
return PyErr_NoMemory();
Expand Down Expand Up @@ -233,7 +234,11 @@ _setimage(ImagingEncoderObject* encoder, PyObject* args)

/* Allocate memory buffer (if bits field is set) */
if (state->bits > 0) {
if (state->xsize > ((INT_MAX / state->bits)-7)) {
return PyErr_NoMemory();
}
state->bytes = (state->bits * state->xsize+7)/8;
/* malloc check ok, overflow checked above */
state->buffer = (UINT8*) malloc(state->bytes);
if (!state->buffer)
return PyErr_NoMemory();
Expand Down Expand Up @@ -478,10 +483,9 @@ PyImaging_ZipEncoderNew(PyObject* self, PyObject* args)
&dictionary, &dictionary_size))
return NULL;

/* Copy to avoid referencing Python's memory, but there's no mechanism to
free this memory later, so this function (and several others here)
leaks. */
/* Copy to avoid referencing Python's memory */
if (dictionary && dictionary_size > 0) {
/* malloc check ok, size comes from PyArg_ParseTuple */
char* p = malloc(dictionary_size);
if (!p)
return PyErr_NoMemory();
Expand All @@ -498,6 +502,7 @@ PyImaging_ZipEncoderNew(PyObject* self, PyObject* args)
return NULL;

encoder->encode = ImagingZipEncode;
encoder->cleanup = ImagingZipEncodeCleanup;

if (rawmode[0] == 'P')
/* disable filtering */
Expand Down Expand Up @@ -559,6 +564,7 @@ static unsigned int* get_qtables_arrays(PyObject* qtables, int* qtablesLen) {
Py_DECREF(tables);
return NULL;
}
/* malloc check ok, num_tables <4, DCTSIZE2 == 64 from jpeglib.h */
qarrays = (unsigned int*) malloc(num_tables * DCTSIZE2 * sizeof(unsigned int));
if (!qarrays) {
Py_DECREF(tables);
Expand Down Expand Up @@ -631,9 +637,11 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args)
if (get_packer(encoder, mode, rawmode) < 0)
return NULL;

// Freed in JpegEncode, Case 5
qarrays = get_qtables_arrays(qtables, &qtablesLen);

if (extra && extra_size > 0) {
/* malloc check ok, length is from python parsearg */
char* p = malloc(extra_size); // Freed in JpegEncode, Case 5
if (!p)
return PyErr_NoMemory();
Expand All @@ -643,6 +651,7 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args)
extra = NULL;

if (rawExif && rawExifLen > 0) {
/* malloc check ok, length is from python parsearg */
char* pp = malloc(rawExifLen); // Freed in JpegEncode, Case 5
if (!pp)
return PyErr_NoMemory();
Expand Down Expand Up @@ -757,15 +766,16 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args)
(ttag_t) PyInt_AsLong(key),
PyBytes_AsString(value));
} else if (PyTuple_Check(value)) {
int len,i;
Py_ssize_t len,i;
float *floatav;
int *intav;
TRACE(("Setting from Tuple: %d \n", (int)PyInt_AsLong(key)));
len = (int)PyTuple_Size(value);
len = PyTuple_Size(value);
if (len) {
if (PyInt_Check(PyTuple_GetItem(value,0))) {
TRACE((" %d elements, setting as ints \n", len));
intav = malloc(sizeof(int)*len);
TRACE((" %d elements, setting as ints \n", (int)len));
/* malloc check ok, calloc checks for overflow */
intav = calloc(len, sizeof(int));
if (intav) {
for (i=0;i<len;i++) {
intav[i] = (int)PyInt_AsLong(PyTuple_GetItem(value,i));
Expand All @@ -776,8 +786,9 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args)
free(intav);
}
} else if (PyFloat_Check(PyTuple_GetItem(value,0))) {
TRACE((" %d elements, setting as floats \n", len));
floatav = malloc(sizeof(float)*len);
TRACE((" %d elements, setting as floats \n", (int)len));
/* malloc check ok, calloc checks for overflow */
floatav = calloc(len, sizeof(float));
if (floatav) {
for (i=0;i<len;i++) {
floatav[i] = (float)PyFloat_AsDouble(PyTuple_GetItem(value,i));
Expand Down
Loading