From c619445003ec60c8166a0b894d8cab49351c886f Mon Sep 17 00:00:00 2001 From: Patryk Obara Date: Sun, 23 Feb 2020 00:41:33 +0100 Subject: [PATCH] Avoid buffer underflow by copying all fields This code made silent assumption that first fields in direntry are exactly 14 bytes - this was fine, except would break as soon as anyone would touch the struct (or e.g. if a compiler would lack support for packed structures and inject some padding in there); rewrite the copy code to follow the same pattern as other fields - now the code will be fine even if someone will change fields in the direntry struct. Fixes 2 PVS static analysis issues (buffer underflow on src and dst). --- include/mem.h | 4 ++++ src/dos/drive_fat.cpp | 21 ++++++++++++--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/include/mem.h b/include/mem.h index da130bb8..e18cbe2e 100644 --- a/include/mem.h +++ b/include/mem.h @@ -127,6 +127,10 @@ static INLINE void host_writed(HostPt off,Bit32u val) { // // __builtin_bswap* is supported since GCC 4.3 and Clang 3.4 +constexpr static INLINE uint8_t host_to_le(uint8_t val) { + return val; +} + #if defined(WORDS_BIGENDIAN) constexpr static INLINE int16_t host_to_le(int16_t val) { diff --git a/src/dos/drive_fat.cpp b/src/dos/drive_fat.cpp index cdad0c30..510851df 100644 --- a/src/dos/drive_fat.cpp +++ b/src/dos/drive_fat.cpp @@ -1098,15 +1098,18 @@ char* trimString(char* str, const size_t max_len) { } static void copyDirEntry(const direntry *src, direntry *dst) { - memcpy(dst, src, 14); // single byte fields - dst->crtTime = host_to_le(src->crtTime); - dst->crtDate = host_to_le(src->crtDate); - dst->accessDate = host_to_le(src->accessDate); - dst->hiFirstClust = host_to_le(src->hiFirstClust); - dst->modTime = host_to_le(src->modTime); - dst->modDate = host_to_le(src->modDate); - dst->loFirstClust = host_to_le(src->loFirstClust); - dst->entrysize = host_to_le(src->entrysize); + memcpy(dst->entryname, src->entryname, sizeof(src->entryname)); + dst->attrib = host_to_le(src->attrib); + dst->NTRes = host_to_le(src->NTRes); + dst->milliSecondStamp = host_to_le(src->milliSecondStamp); + dst->crtTime = host_to_le(src->crtTime); + dst->crtDate = host_to_le(src->crtDate); + dst->accessDate = host_to_le(src->accessDate); + dst->hiFirstClust = host_to_le(src->hiFirstClust); + dst->modTime = host_to_le(src->modTime); + dst->modDate = host_to_le(src->modDate); + dst->loFirstClust = host_to_le(src->loFirstClust); + dst->entrysize = host_to_le(src->entrysize); } bool fatDrive::FindNextInternal(Bit32u dirClustNumber, DOS_DTA &dta, direntry *foundEntry) {