Fix undefined behaviour when reading corrupted ZIP files

Don't shift by m_SystemMadeBy value which can potentially be an
arbitrary (8 bit) integer and not necessarily one of the known (and
small) wxZIP_SYSTEM_XXX values, this results in undefined behaviour
whenever this value is greater than 32 or 64 (depending on int size) and
is flagged as such by clang undefined behaviour sanitizer.

To fix the problem, just use a more clear switch statement instead of
using a bit pattern for the lookup, this function is not nearly
performance-sensitive enough to worry about the overhead of the switch
here (assuming it's even slower, in the first place...) and the new
version is much more clear and maintainable.

Credit to OSS-Fuzz: this solves its issue 3792.
This commit is contained in:
Vadim Zeitlin 2017-10-25 16:38:37 +02:00
parent d5a6568b21
commit 802eac475d

View File

@ -517,17 +517,37 @@ inline bool wxZipEntry::IsReadOnly() const
inline bool wxZipEntry::IsMadeByUnix() const
{
const int pattern =
(1 << wxZIP_SYSTEM_OPENVMS) |
(1 << wxZIP_SYSTEM_UNIX) |
(1 << wxZIP_SYSTEM_ATARI_ST) |
(1 << wxZIP_SYSTEM_ACORN_RISC) |
(1 << wxZIP_SYSTEM_BEOS) | (1 << wxZIP_SYSTEM_TANDEM);
switch ( m_SystemMadeBy )
{
case wxZIP_SYSTEM_MSDOS:
// note: some unix zippers put madeby = dos
return m_ExternalAttributes & ~0xFFFF;
// note: some unix zippers put madeby = dos
return (m_SystemMadeBy == wxZIP_SYSTEM_MSDOS
&& (m_ExternalAttributes & ~0xFFFF))
|| ((pattern >> m_SystemMadeBy) & 1);
case wxZIP_SYSTEM_OPENVMS:
case wxZIP_SYSTEM_UNIX:
case wxZIP_SYSTEM_ATARI_ST:
case wxZIP_SYSTEM_ACORN_RISC:
case wxZIP_SYSTEM_BEOS:
case wxZIP_SYSTEM_TANDEM:
return true;
case wxZIP_SYSTEM_AMIGA:
case wxZIP_SYSTEM_VM_CMS:
case wxZIP_SYSTEM_OS2_HPFS:
case wxZIP_SYSTEM_MACINTOSH:
case wxZIP_SYSTEM_Z_SYSTEM:
case wxZIP_SYSTEM_CPM:
case wxZIP_SYSTEM_WINDOWS_NTFS:
case wxZIP_SYSTEM_MVS:
case wxZIP_SYSTEM_VSE:
case wxZIP_SYSTEM_VFAT:
case wxZIP_SYSTEM_ALTERNATE_MVS:
case wxZIP_SYSTEM_OS_400:
return false;
}
// Unknown system, assume not Unix.
return false;
}
inline void wxZipEntry::SetIsText(bool isText)