diff --git a/lib/kodi-libmpo-note.txt b/lib/kodi-libmpo-note.txt index 2d36e77..80c7d1f 100644 --- a/lib/kodi-libmpo-note.txt +++ b/lib/kodi-libmpo-note.txt @@ -1,2 +1,7 @@ libmpo source from https://github.com/Lectem/libmpo -Sync to 63ada10 (14 Aug 2017) \ No newline at end of file +Sync to 63ada10 (14 Aug 2017) + +Locally modified, not present upstream (upstream's last code commit is 2017): + src/mpo.c - mpf_getbyte(), mpf_seek() and mpf_dc_rewindc() bounds-check the + cursor. The only check was an assert() in mpf_getbyte(), which NDEBUG removes + from release builds, leaving every read in the MP parser unchecked. diff --git a/lib/libmpo/include/libmpo/mpo.h b/lib/libmpo/include/libmpo/mpo.h index 337bf81..519df9f 100644 --- a/lib/libmpo/include/libmpo/mpo.h +++ b/lib/libmpo/include/libmpo/mpo.h @@ -46,6 +46,9 @@ typedef struct {MPFLong numerator; MPFLong denominator;} MPFRational; typedef unsigned char MPFUndefined; /* 1 byte */ typedef struct {MPFSLong numerator; MPFSLong denominator;} MPFSRational; +/* Kodi downstream: a sanity ceiling on the file-supplied image count. */ +#define MPO_MAX_IMAGES 64 + typedef struct { MPFByte * buffer; diff --git a/lib/libmpo/src/dmpo.c b/lib/libmpo/src/dmpo.c index 4ef7f60..5ab407c 100644 --- a/lib/libmpo/src/dmpo.c +++ b/lib/libmpo/src/dmpo.c @@ -122,8 +122,34 @@ bool mpo_read_header(mpo_decompress_struct *mpoinfo) { res = jpeg_read_header((j_decompress_ptr) &mpoinfo->cinfo, TRUE) != 0; int nbImages = mpoinfo->APP02->numberOfImages; - if(nbImages > 1) - mpoinfo->APP02 = realloc(mpoinfo->APP02,nbImages * (sizeof *mpoinfo->APP02)); + /* Kodi downstream: numberOfImages comes from the file, and + mpo_destroy_decompress() walks every entry calling free() on its + MPentry. realloc() does not zero what it adds, and only entry 0 has + been parsed at this point, so the rest must be cleared or the + teardown frees uninitialised pointers. */ + if(nbImages > 1 && nbImages <= MPO_MAX_IMAGES) + { + MPExt_Data *grown = realloc(mpoinfo->APP02,nbImages * (sizeof *mpoinfo->APP02)); + if(grown) + { + memset(grown + 1, 0, (nbImages - 1) * (sizeof *grown)); + mpoinfo->APP02 = grown; + } + else + { + /* Only one entry is allocated, and mpo_destroy_decompress() + walks numberOfImages of them, so it has to agree. */ + mpoinfo->APP02->numberOfImages = 1; + res = 0; + } + } + else if(nbImages > MPO_MAX_IMAGES) + { + mpoinfo->APP02->numberOfImages = 1; + res = 0; + } + else if(nbImages < 1) + mpoinfo->APP02->numberOfImages = 1; } return res; diff --git a/lib/libmpo/src/mpo.c b/lib/libmpo/src/mpo.c index d007a2d..9382e0d 100644 --- a/lib/libmpo/src/mpo.c +++ b/lib/libmpo/src/mpo.c @@ -20,21 +20,29 @@ long mpf_tell(MPFbuffer_ptr b) void mpf_seek(MPFbuffer_ptr b,long offset, int from) { - if(from==SEEK_CUR)b->_cur+=offset; - else if(from==SEEK_SET)b->_cur=offset; - else if(from==SEEK_END)b->_cur=b->_size-1+offset; + /* Kodi downstream: offsets come from the file, so clamp rather than trust. */ + long target; + if(from==SEEK_CUR)target=b->_cur+offset; + else if(from==SEEK_SET)target=offset; + else if(from==SEEK_END)target=b->_size-1+offset; + else return; + if(target<0)target=0; + if(target>b->_size)target=b->_size; + b->_cur=target; } unsigned int mpf_getbyte (MPFbuffer_ptr b) /* Read next byte */ { - assert(b->_cur < b->_size); + /* Kodi downstream: this assert was the only bounds check on every read in + the MP parser, and NDEBUG removes it from the builds we ship. */ + if(b->_cur < 0 || b->_cur >= b->_size)return 0; return b->buffer[b->_cur++]; } void mpf_dc_rewindc(MPFbuffer_ptr b) { - b->_cur--; + if(b->_cur>0)b->_cur--; } uint16_t mpf_getint16 (MPFbuffer_ptr b, int swapEndian) diff --git a/src/MPOPicture.cpp b/src/MPOPicture.cpp index 21dd72b..cdc4280 100644 --- a/src/MPOPicture.cpp +++ b/src/MPOPicture.cpp @@ -10,9 +10,30 @@ #include "../lib/TinyEXIF/TinyEXIF.h" +#include #include #include +namespace +{ + +// libjpeg's default error_exit calls exit(), and libmpo installs it via +// jpeg_std_error() without overriding it, so a malformed file terminates Kodi +// rather than failing the decode. mpo_decompress_error_exit() can only replace +// the function pointer, not attach state to it, so the jump target is +// thread-local; each decoder instance is driven from one thread at a time. +thread_local std::jmp_buf s_jpegEscape; + +void MpoFatalError(j_common_ptr cinfo) +{ + char message[JMSG_LENGTH_MAX] = {}; + (*cinfo->err->format_message)(cinfo, message); + kodi::Log(ADDON_LOG_ERROR, "libjpeg: %s", message); + std::longjmp(s_jpegEscape, 1); +} + +} // namespace + MPOPicture::MPOPicture(const kodi::addon::IInstanceInfo& instance) : CInstanceImageDecoder(instance) { @@ -36,8 +57,15 @@ bool MPOPicture::SupportsFile(const std::string& file) mpo_decompress_struct mpoinfo; mpo_create_decompress(&mpoinfo); + mpo_decompress_error_exit(&mpoinfo, MpoFatalError); + if (setjmp(s_jpegEscape)) + { + mpo_destroy_decompress(&mpoinfo); + return false; + } + mpo_mem_src(&mpoinfo, buffer.data(), buffer.size()); - bool ret = mpo_read_header(&mpoinfo); + const bool ret = mpo_read_header(&mpoinfo); mpo_destroy_decompress(&mpoinfo); return ret; } @@ -139,6 +167,13 @@ bool MPOPicture::LoadImageFromMemory(const std::string& mimetype, m_data.resize(bufSize); std::copy(buffer, buffer + bufSize, m_data.begin()); mpo_create_decompress(&m_mpoinfo); + mpo_decompress_error_exit(&m_mpoinfo, MpoFatalError); + if (setjmp(s_jpegEscape)) + { + mpo_destroy_decompress(&m_mpoinfo); + return false; + } + mpo_mem_src(&m_mpoinfo, m_data.data(), m_data.size()); if (!mpo_read_header(&m_mpoinfo)) { @@ -169,6 +204,9 @@ bool MPOPicture::Decode(uint8_t* pixels, return false; } + if (setjmp(s_jpegEscape)) + return false; + size_t image = 0; while (image < m_images) {