From a431d7806f5deccf60767aad686cd829147aa89d Mon Sep 17 00:00:00 2001 From: Michael R Sweet Date: Mon, 29 Nov 2021 17:46:56 -0500 Subject: [PATCH] Fix a few stack/buffer overflow bugs discovered by Bart, Steffan, and Mark from the Radboud University NL (thanks!) - Add depth argument to all value read functions that recurse - Add depth argument to page tree loading code - Validate xref stream sizes individually to avoid out-of-bounds access to local xref buffer. --- CHANGES.md | 6 ++++++ Makefile | 2 +- pdfio-array.c | 5 +++-- pdfio-dict.c | 5 +++-- pdfio-file.c | 23 +++++++++++++++-------- pdfio-object.c | 2 +- pdfio-private.h | 8 +++++--- pdfio-value.c | 19 ++++++++++++++++--- 8 files changed, 50 insertions(+), 20 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 00a4276..a576d06 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,12 @@ Changes in PDFio ================ +v1.0rc1 (Month DD, YYYY) +------------------------ + +- Fixed a few stack/buffer overflow bugs discovered via fuzzing. + + v1.0b2 (November 7, 2021) ------------------------- diff --git a/Makefile b/Makefile index 465dd5d..4c0a92b 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ CC = cc CFLAGS = CODESIGN_IDENTITY = Developer ID #COMMONFLAGS = -Os -g -COMMONFLAGS = -O0 -g +COMMONFLAGS = -O0 -g -fsanitize=address CPPFLAGS = '-DPDFIO_VERSION="$(VERSION)"' DESTDIR = $(DSTROOT) DSO = cc diff --git a/pdfio-array.c b/pdfio-array.c index ac303d0..789fcc7 100644 --- a/pdfio-array.c +++ b/pdfio-array.c @@ -575,7 +575,8 @@ _pdfioArrayGetValue(pdfio_array_t *a, // I - Array pdfio_array_t * // O - New array _pdfioArrayRead(pdfio_file_t *pdf, // I - PDF file pdfio_obj_t *obj, // I - Object, if any - _pdfio_token_t *tb) // I - Token buffer/stack + _pdfio_token_t *tb, // I - Token buffer/stack + size_t depth) // I - Depth of array { pdfio_array_t *array; // New array char token[8192]; // Token from file @@ -599,7 +600,7 @@ _pdfioArrayRead(pdfio_file_t *pdf, // I - PDF file // Push the token and decode the value... _pdfioTokenPush(tb, token); - if (!_pdfioValueRead(pdf, obj, tb, &value)) + if (!_pdfioValueRead(pdf, obj, tb, &value, depth)) break; // PDFIO_DEBUG("_pdfioArrayRead(%p): Appending ", (void *)array); diff --git a/pdfio-dict.c b/pdfio-dict.c index c01fc6e..a5ae739 100644 --- a/pdfio-dict.c +++ b/pdfio-dict.c @@ -473,7 +473,8 @@ _pdfioDictGetValue(pdfio_dict_t *dict, // I - Dictionary pdfio_dict_t * // O - New dictionary _pdfioDictRead(pdfio_file_t *pdf, // I - PDF file pdfio_obj_t *obj, // I - Object, if any - _pdfio_token_t *tb) // I - Token buffer/stack + _pdfio_token_t *tb, // I - Token buffer/stack + size_t depth) // I - Depth of dictionary { pdfio_dict_t *dict; // New dictionary char key[256]; // Dictionary key @@ -501,7 +502,7 @@ _pdfioDictRead(pdfio_file_t *pdf, // I - PDF file } // Then get the next value... - if (!_pdfioValueRead(pdf, obj, tb, &value)) + if (!_pdfioValueRead(pdf, obj, tb, &value, depth)) { _pdfioFileError(pdf, "Missing value for dictionary key."); break; diff --git a/pdfio-file.c b/pdfio-file.c index eab2640..b907013 100644 --- a/pdfio-file.c +++ b/pdfio-file.c @@ -25,7 +25,7 @@ static pdfio_obj_t *add_obj(pdfio_file_t *pdf, size_t number, unsigned short gen static int compare_objmaps(_pdfio_objmap_t *a, _pdfio_objmap_t *b); static int compare_objs(pdfio_obj_t **a, pdfio_obj_t **b); static bool load_obj_stream(pdfio_obj_t *obj); -static bool load_pages(pdfio_file_t *pdf, pdfio_obj_t *obj); +static bool load_pages(pdfio_file_t *pdf, pdfio_obj_t *obj, size_t depth); static bool load_xref(pdfio_file_t *pdf, off_t xref_offset, pdfio_password_cb_t password_cb, void *password_data); static bool write_catalog(pdfio_file_t *pdf); static bool write_pages(pdfio_file_t *pdf); @@ -1312,7 +1312,7 @@ load_obj_stream(pdfio_obj_t *obj) // I - Object to load // Read the objects themselves... for (cur_obj = 0; cur_obj < num_objs; cur_obj ++) { - if (!_pdfioValueRead(obj->pdf, obj, &tb, &(objs[cur_obj]->value))) + if (!_pdfioValueRead(obj->pdf, obj, &tb, &(objs[cur_obj]->value), 0)) { pdfioStreamClose(st); return (false); @@ -1332,7 +1332,8 @@ load_obj_stream(pdfio_obj_t *obj) // I - Object to load static bool // O - `true` on success, `false` on error load_pages(pdfio_file_t *pdf, // I - PDF file - pdfio_obj_t *obj) // I - Page object + pdfio_obj_t *obj, // I - Page object + size_t depth) // I - Depth of page tree { pdfio_dict_t *dict; // Page object dictionary const char *type; // Node type @@ -1364,9 +1365,15 @@ load_pages(pdfio_file_t *pdf, // I - PDF file size_t i, // Looping var num_kids; // Number of elements in array + if (depth >= PDFIO_MAX_DEPTH) + { + _pdfioFileError(pdf, "Depth of pages objects too great to load."); + return (false); + } + for (i = 0, num_kids = pdfioArrayGetSize(kids); i < num_kids; i ++) { - if (!load_pages(pdf, pdfioArrayGetObj(kids, i))) + if (!load_pages(pdf, pdfioArrayGetObj(kids, i), depth + 1)) return (false); } } @@ -1496,7 +1503,7 @@ load_xref( _pdfioTokenInit(&tb, pdf, (_pdfio_tconsume_cb_t)_pdfioFileConsume, (_pdfio_tpeek_cb_t)_pdfioFilePeek, pdf); - if (!_pdfioValueRead(pdf, obj, &tb, &trailer)) + if (!_pdfioValueRead(pdf, obj, &tb, &trailer, 0)) { _pdfioFileError(pdf, "Unable to read cross-reference stream dictionary."); return (false); @@ -1537,7 +1544,7 @@ load_xref( w_2 = w[0]; w_3 = w[0] + w[1]; - if (w[1] == 0 || w[2] > 2 || w_total > sizeof(buffer)) + if (w[1] == 0 || w[2] > 2 || w[0] > sizeof(buffer) || w[1] > sizeof(buffer) || w[2] > sizeof(buffer) || w_total > sizeof(buffer)) { _pdfioFileError(pdf, "Cross-reference stream has invalid W key."); return (false); @@ -1751,7 +1758,7 @@ load_xref( _pdfioTokenInit(&tb, pdf, (_pdfio_tconsume_cb_t)_pdfioFileConsume, (_pdfio_tpeek_cb_t)_pdfioFilePeek, pdf); - if (!_pdfioValueRead(pdf, NULL, &tb, &trailer)) + if (!_pdfioValueRead(pdf, NULL, &tb, &trailer, 0)) { _pdfioFileError(pdf, "Unable to read trailer dictionary."); return (false); @@ -1803,7 +1810,7 @@ load_xref( PDFIO_DEBUG("load_xref: Root=%p(%lu)\n", pdf->root_obj, (unsigned long)pdf->root_obj->number); - return (load_pages(pdf, pdfioDictGetObj(pdfioObjGetDict(pdf->root_obj), "Pages"))); + return (load_pages(pdf, pdfioDictGetObj(pdfioObjGetDict(pdf->root_obj), "Pages"), 0)); } diff --git a/pdfio-object.c b/pdfio-object.c index f7dab8c..d919982 100644 --- a/pdfio-object.c +++ b/pdfio-object.c @@ -412,7 +412,7 @@ _pdfioObjLoad(pdfio_obj_t *obj) // I - Object // Then grab the object value... _pdfioTokenInit(&tb, obj->pdf, (_pdfio_tconsume_cb_t)_pdfioFileConsume, (_pdfio_tpeek_cb_t)_pdfioFilePeek, obj->pdf); - if (!_pdfioValueRead(obj->pdf, obj, &tb, &obj->value)) + if (!_pdfioValueRead(obj->pdf, obj, &tb, &obj->value, 0)) { _pdfioFileError(obj->pdf, "Unable to read value for object %lu.", (unsigned long)obj->number); return (false); diff --git a/pdfio-private.h b/pdfio-private.h index 8140582..9c59c75 100644 --- a/pdfio-private.h +++ b/pdfio-private.h @@ -116,6 +116,8 @@ // Types and constants... // +# define PDFIO_MAX_DEPTH 32 // Maximum nesting depth for values + typedef enum _pdfio_mode_e // Read/write mode { _PDFIO_MODE_READ, // Read a PDF file @@ -341,7 +343,7 @@ struct _pdfio_stream_s // Stream extern void _pdfioArrayDebug(pdfio_array_t *a, FILE *fp) _PDFIO_INTERNAL; extern void _pdfioArrayDelete(pdfio_array_t *a) _PDFIO_INTERNAL; extern _pdfio_value_t *_pdfioArrayGetValue(pdfio_array_t *a, size_t n) _PDFIO_INTERNAL; -extern pdfio_array_t *_pdfioArrayRead(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_token_t *ts) _PDFIO_INTERNAL; +extern pdfio_array_t *_pdfioArrayRead(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_token_t *ts, size_t depth) _PDFIO_INTERNAL; extern bool _pdfioArrayWrite(pdfio_array_t *a, pdfio_obj_t *obj) _PDFIO_INTERNAL; extern void _pdfioCryptoAESInit(_pdfio_aes_t *ctx, const uint8_t *key, size_t keylen, const uint8_t *iv) _PDFIO_INTERNAL; @@ -365,7 +367,7 @@ extern void _pdfioDictClear(pdfio_dict_t *dict, const char *key) _PDFIO_INTERNA extern void _pdfioDictDebug(pdfio_dict_t *dict, FILE *fp) _PDFIO_INTERNAL; extern void _pdfioDictDelete(pdfio_dict_t *dict) _PDFIO_INTERNAL; extern _pdfio_value_t *_pdfioDictGetValue(pdfio_dict_t *dict, const char *key) _PDFIO_INTERNAL; -extern pdfio_dict_t *_pdfioDictRead(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_token_t *ts) _PDFIO_INTERNAL; +extern pdfio_dict_t *_pdfioDictRead(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_token_t *ts, size_t depth) _PDFIO_INTERNAL; extern bool _pdfioDictSetValue(pdfio_dict_t *dict, const char *key, _pdfio_value_t *value) _PDFIO_INTERNAL; extern bool _pdfioDictWrite(pdfio_dict_t *dict, pdfio_obj_t *obj, off_t *length) _PDFIO_INTERNAL; @@ -405,7 +407,7 @@ extern bool _pdfioTokenRead(_pdfio_token_t *tb, char *buffer, size_t bufsize); extern _pdfio_value_t *_pdfioValueCopy(pdfio_file_t *pdfdst, _pdfio_value_t *vdst, pdfio_file_t *pdfsrc, _pdfio_value_t *vsrc) _PDFIO_INTERNAL; extern void _pdfioValueDebug(_pdfio_value_t *v, FILE *fp) _PDFIO_INTERNAL; extern void _pdfioValueDelete(_pdfio_value_t *v) _PDFIO_INTERNAL; -extern _pdfio_value_t *_pdfioValueRead(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_token_t *ts, _pdfio_value_t *v) _PDFIO_INTERNAL; +extern _pdfio_value_t *_pdfioValueRead(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_token_t *ts, _pdfio_value_t *v, size_t depth) _PDFIO_INTERNAL; extern bool _pdfioValueWrite(pdfio_file_t *pdf, pdfio_obj_t *obj, _pdfio_value_t *v, off_t *length) _PDFIO_INTERNAL; #endif // !PDFIO_PRIVATE_H diff --git a/pdfio-value.c b/pdfio-value.c index eef0278..25e293d 100644 --- a/pdfio-value.c +++ b/pdfio-value.c @@ -196,7 +196,8 @@ _pdfio_value_t * // O - Value or `NULL` on error/EOF _pdfioValueRead(pdfio_file_t *pdf, // I - PDF file pdfio_obj_t *obj, // I - Object, if any _pdfio_token_t *tb, // I - Token buffer/stack - _pdfio_value_t *v) // I - Value + _pdfio_value_t *v, // I - Value + size_t depth) // I - Depth of value { char token[32768]; // Token buffer #ifdef DEBUG @@ -226,15 +227,27 @@ _pdfioValueRead(pdfio_file_t *pdf, // I - PDF file if (!strcmp(token, "[")) { // Start of array + if (depth >= PDFIO_MAX_DEPTH) + { + _pdfioFileError(pdf, "Too many nested arrays."); + return (NULL); + } + v->type = PDFIO_VALTYPE_ARRAY; - if ((v->value.array = _pdfioArrayRead(pdf, obj, tb)) == NULL) + if ((v->value.array = _pdfioArrayRead(pdf, obj, tb, depth + 1)) == NULL) return (NULL); } else if (!strcmp(token, "<<")) { // Start of dictionary + if (depth >= PDFIO_MAX_DEPTH) + { + _pdfioFileError(pdf, "Too many nested dictionaries."); + return (NULL); + } + v->type = PDFIO_VALTYPE_DICT; - if ((v->value.dict = _pdfioDictRead(pdf, obj, tb)) == NULL) + if ((v->value.dict = _pdfioDictRead(pdf, obj, tb, depth + 1)) == NULL) return (NULL); } else if (!strncmp(token, "(D:", 3))