1
0
Fork 0

Fix unsafe memory operations and warnings in the fatDrive class

- Move imageDiskList from pointer to vector of unique_ptr
- Replace string operations with size-limited versions
- Initialize members
- Eliminate unecessary casts
- Eliminate memory-leak on pointer assignment
This commit is contained in:
krcroft 2019-12-07 11:55:38 -08:00 committed by Patryk Obara
parent e942a02fcb
commit c9198b2944
11 changed files with 219 additions and 131 deletions

View file

@ -69,7 +69,7 @@ static const char* UnmountHelper(char umount) {
if (i_drive >= DOS_DRIVES || i_drive < 0)
return MSG_Get("PROGRAM_MOUNT_UMOUNT_NOT_MOUNTED");
if (i_drive < MAX_DISK_IMAGES && Drives[i_drive] == NULL && imageDiskList[i_drive] == NULL)
if (i_drive < MAX_DISK_IMAGES && Drives[i_drive] == NULL && !imageDiskList[i_drive])
return MSG_Get("PROGRAM_MOUNT_UMOUNT_NOT_MOUNTED");
if (i_drive >= MAX_DISK_IMAGES && Drives[i_drive] == NULL)
@ -89,8 +89,7 @@ static const char* UnmountHelper(char umount) {
}
if (i_drive < MAX_DISK_IMAGES && imageDiskList[i_drive]) {
delete imageDiskList[i_drive];
imageDiskList[i_drive] = NULL;
imageDiskList[i_drive].reset(nullptr);
}
return MSG_Get("PROGRAM_MOUNT_UMOUNT_SUCCESS");
@ -717,8 +716,7 @@ public:
Bit32u rombytesize;
FILE *usefile = getFSFile(temp_line.c_str(), &floppysize, &rombytesize);
if(usefile != NULL) {
if(diskSwap[i] != NULL) delete diskSwap[i];
diskSwap[i] = new imageDisk(usefile, temp_line.c_str(), floppysize, false);
diskSwap[i].reset(new imageDisk(usefile, temp_line.c_str(), floppysize, false));
if (usefile_1==NULL) {
usefile_1=usefile;
rombytesize_1=rombytesize;
@ -739,7 +737,7 @@ public:
swapInDisks();
if(imageDiskList[drive-65]==NULL) {
if(!imageDiskList[drive-65]) {
WriteOut(MSG_Get("PROGRAM_BOOT_UNABLE"), drive);
return;
}
@ -778,10 +776,7 @@ public:
WriteOut(MSG_Get("PROGRAM_BOOT_CART_NO_CMDS"));
}
for(Bitu dct=0;dct<MAX_SWAPPABLE_DISKS;dct++) {
if(diskSwap[dct]!=NULL) {
delete diskSwap[dct];
diskSwap[dct]=NULL;
}
diskSwap[dct].reset(nullptr);
}
//fclose(usefile_1); //delete diskSwap closes the file
return;
@ -810,10 +805,7 @@ public:
WriteOut(MSG_Get("PROGRAM_BOOT_CART_NO_CMDS"));
}
for(Bitu dct=0;dct<MAX_SWAPPABLE_DISKS;dct++) {
if(diskSwap[dct]!=NULL) {
delete diskSwap[dct];
diskSwap[dct]=NULL;
}
diskSwap[dct].reset(nullptr);
}
//fclose(usefile_1); //Delete diskSwap closes the file
return;
@ -866,10 +858,7 @@ public:
//Close cardridges
for(Bitu dct=0;dct<MAX_SWAPPABLE_DISKS;dct++) {
if(diskSwap[dct]!=NULL) {
delete diskSwap[dct];
diskSwap[dct]=NULL;
}
diskSwap[dct].reset(nullptr);
}
@ -1450,15 +1439,13 @@ public:
case 0:
case 1:
if(!((fatDrive *)newdrive)->loadedDisk->hardDrive) {
if(imageDiskList[drive - 'A'] != NULL) delete imageDiskList[drive - 'A'];
imageDiskList[drive - 'A'] = ((fatDrive *)newdrive)->loadedDisk;
imageDiskList[drive - 'A'].reset(((fatDrive *)newdrive)->loadedDisk.get());
}
break;
case 2:
case 3:
if(((fatDrive *)newdrive)->loadedDisk->hardDrive) {
if(imageDiskList[drive - 'A'] != NULL) delete imageDiskList[drive - 'A'];
imageDiskList[drive - 'A'] = ((fatDrive *)newdrive)->loadedDisk;
imageDiskList[drive - 'A'].reset(((fatDrive *)newdrive)->loadedDisk.get());
updateDPT();
}
break;
@ -1534,8 +1521,7 @@ public:
imageDisk * newImage = new imageDisk(newDisk, temp_line.c_str(), imagesize, hdd);
if (hdd) newImage->Set_Geometry(sizes[2],sizes[3],sizes[1],sizes[0]);
if(imageDiskList[drive - '0'] != NULL) delete imageDiskList[drive - '0'];
imageDiskList[drive - '0'] = newImage;
imageDiskList[drive - '0'].reset(newImage);
updateDPT();
WriteOut(MSG_Get("PROGRAM_IMGMOUNT_MOUNT_NUMBER"),drive - '0',temp_line.c_str());
}

View file

@ -21,6 +21,7 @@
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include "dosbox.h"
#include "dos_inc.h"
#include "drives.h"
@ -40,6 +41,8 @@
class fatFile : public DOS_File {
public:
fatFile(const char* name, Bit32u startCluster, Bit32u fileLen, fatDrive *useDrive);
fatFile(const fatFile&) = delete; // prevent copy
fatFile& operator=(const fatFile&) = delete; // prevent assignment
bool Read(Bit8u * data,Bit16u * size);
bool Write(Bit8u * data,Bit16u * size);
bool Seek(Bit32u * pos,Bit32u type);
@ -67,7 +70,7 @@ public:
static void convToDirFile(char *filename, char *filearray) {
Bit32u charidx = 0;
Bit32u flen,i;
flen = (Bit32u)strlen(filename);
flen = (Bit32u)strnlen(filename, DOS_NAMELENGTH_ASCII);
memset(filearray, 32, 11);
for(i=0;i<flen;i++) {
if(charidx >= 11) break;
@ -80,17 +83,23 @@ static void convToDirFile(char *filename, char *filearray) {
}
}
fatFile::fatFile(const char* /*name*/, Bit32u startCluster, Bit32u fileLen, fatDrive *useDrive) {
fatFile::fatFile(const char* /*name*/,
Bit32u startCluster,
Bit32u fileLen,
fatDrive *useDrive)
: firstCluster(startCluster),
seekpos(0),
filelength(fileLen),
currentSector(0),
curSectOff(0),
sectorBuffer{0},
dirCluster(0),
dirIndex(0),
loadedSector(false),
myDrive(useDrive)
{
Bit32u seekto = 0;
firstCluster = startCluster;
myDrive = useDrive;
filelength = fileLen;
open = true;
loadedSector = false;
curSectOff = 0;
seekpos = 0;
memset(&sectorBuffer[0], 0, sizeof(sectorBuffer));
if(filelength > 0) {
Seek(&seekto, DOS_SEEK_SET);
}
@ -413,7 +422,7 @@ bool fatDrive::getEntryName(char *fullname, char *entname) {
char * findDir;
char * findFile;
strcpy(dirtoken,fullname);
safe_strcpy(dirtoken, fullname);
//LOG_MSG("Testing for filename %s", fullname);
findDir = strtok(dirtoken,"\\");
@ -425,19 +434,19 @@ bool fatDrive::getEntryName(char *fullname, char *entname) {
findFile = findDir;
findDir = strtok(NULL,"\\");
}
strcpy(entname, findFile);
strncpy(entname, findFile, DOS_NAMELENGTH_ASCII);
return true;
}
bool fatDrive::getFileDirEntry(char const * const filename, direntry * useEntry, Bit32u * dirClust, Bit32u * subEntry) {
size_t len = strlen(filename);
size_t len = strnlen(filename, DOS_PATHLENGTH);
char dirtoken[DOS_PATHLENGTH];
Bit32u currentClust = 0;
direntry foundEntry;
char * findDir;
char * findFile;
strcpy(dirtoken,filename);
safe_strcpy(dirtoken, filename);
findFile=dirtoken;
/* Skip if testing in root directory */
@ -471,18 +480,18 @@ bool fatDrive::getFileDirEntry(char const * const filename, direntry * useEntry,
if(!FindNextInternal(currentClust, *imgDTA, &foundEntry)) return false;
memcpy(useEntry, &foundEntry, sizeof(direntry));
*dirClust = (Bit32u)currentClust;
*dirClust = currentClust;
*subEntry = ((Bit32u)imgDTA->GetDirID()-1);
return true;
}
bool fatDrive::getDirClustNum(char *dir, Bit32u *clustNum, bool parDir) {
Bit32u len = (Bit32u)strlen(dir);
Bit32u len = (Bit32u)strnlen(dir, DOS_PATHLENGTH);
char dirtoken[DOS_PATHLENGTH];
Bit32u currentClust = 0;
direntry foundEntry;
char * findDir;
strcpy(dirtoken,dir);
safe_strcpy(dirtoken, dir);
/* Skip if testing for root directory */
if ((len>0) && (dir[len-1]!='\\')) {
@ -513,17 +522,31 @@ bool fatDrive::getDirClustNum(char *dir, Bit32u *clustNum, bool parDir) {
}
Bit8u fatDrive::readSector(Bit32u sectnum, void * data) {
if (absolute) return loadedDisk->Read_AbsoluteSector(sectnum, data);
// Guard
if (!loadedDisk) {
return 0;
}
if (absolute) {
return loadedDisk->Read_AbsoluteSector(sectnum, data);
}
Bit32u cylindersize = bootbuffer.headcount * bootbuffer.sectorspertrack;
Bit32u cylinder = sectnum / cylindersize;
sectnum %= cylindersize;
Bit32u head = sectnum / bootbuffer.sectorspertrack;
Bit32u sector = sectnum % bootbuffer.sectorspertrack + 1L;
return loadedDisk->Read_Sector(head, cylinder, sector, data);
}
}
Bit8u fatDrive::writeSector(Bit32u sectnum, void * data) {
if (absolute) return loadedDisk->Write_AbsoluteSector(sectnum, data);
// Guard
if (!loadedDisk) {
return 0;
}
if (absolute) {
return loadedDisk->Write_AbsoluteSector(sectnum, data);
}
Bit32u cylindersize = bootbuffer.headcount * bootbuffer.sectorspertrack;
Bit32u cylinder = sectnum / cylindersize;
sectnum %= cylindersize;
@ -689,8 +712,28 @@ bool fatDrive::allocateCluster(Bit32u useCluster, Bit32u prevCluster) {
return true;
}
fatDrive::fatDrive(const char *sysFilename, Bit32u bytesector, Bit32u cylsector, Bit32u headscyl, Bit32u cylinders, Bit32u startSector) {
created_successfully = true;
fatDrive::fatDrive(const char *sysFilename,
Bit32u bytesector,
Bit32u cylsector,
Bit32u headscyl,
Bit32u cylinders,
Bit32u startSector)
: loadedDisk(nullptr),
created_successfully(true),
srchInfo{ {0} },
allocation{0, 0, 0, 0, 0},
bootbuffer{{0}, {0}, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, {0}, 0, 0},
absolute(false),
fattype(0),
CountOfClusters(0),
partSectOff(0),
firstDataSector(0),
firstRootDirSect(0),
cwdDirCluster(0),
dirPosition(0),
fatSectBuffer{0},
curFatSect(0)
{
FILE *diskfile;
Bit32u filesize;
bool is_hdd;
@ -703,13 +746,16 @@ fatDrive::fatDrive(const char *sysFilename, Bit32u bytesector, Bit32u cylsector,
}
diskfile = fopen_wrap(sysFilename, "rb+");
if(!diskfile) {created_successfully = false;return;}
if(!diskfile) {
created_successfully = false;
return;
}
fseek(diskfile, 0L, SEEK_END);
filesize = (Bit32u)ftell(diskfile) / 1024L;
is_hdd = (filesize > 2880);
/* Load disk image */
loadedDisk = new imageDisk(diskfile, sysFilename, filesize, is_hdd);
loadedDisk.reset(new imageDisk(diskfile, sysFilename, filesize, is_hdd));
if(!loadedDisk) {
created_successfully = false;
return;
@ -861,30 +907,43 @@ fatDrive::fatDrive(const char *sysFilename, Bit32u bytesector, Bit32u cylsector,
memset(fatSectBuffer,0,1024);
curFatSect = 0xffffffff;
strcpy(info, "fatDrive ");
strcat(info, sysFilename);
safe_strcpy(info, "fatDrive ");
safe_strcat(info, sysFilename);
}
bool fatDrive::AllocationInfo(Bit16u *_bytes_sector, Bit8u *_sectors_cluster, Bit16u *_total_clusters, Bit16u *_free_clusters) {
// Guard
if (!loadedDisk) {
return false;
}
Bit32u hs, cy, sect,sectsize;
Bit32u countFree = 0;
Bit32u i;
loadedDisk->Get_Geometry(&hs, &cy, &sect, &sectsize);
*_bytes_sector = (Bit16u)sectsize;
*_sectors_cluster = bootbuffer.sectorspercluster;
if (CountOfClusters<65536) *_total_clusters = (Bit16u)CountOfClusters;
else {
if (CountOfClusters<65536) {
*_total_clusters = (Bit16u)CountOfClusters;
} else {
// maybe some special handling needed for fat32
*_total_clusters = 65535;
}
for(i=0;i<CountOfClusters;i++) if(!getClusterValue(i+2)) countFree++;
if (countFree<65536) *_free_clusters = (Bit16u)countFree;
else {
for(i=0;i<CountOfClusters;i++) {
if(!getClusterValue(i+2)) {
countFree++;
}
}
if (countFree<65536) {
*_free_clusters = (Bit16u)countFree;
} else {
// maybe some special handling needed for fat32
*_free_clusters = 65535;
}
return true;
}
@ -906,13 +965,16 @@ Bits fatDrive::UnMount(void) {
return 0;
}
Bit8u fatDrive::GetMediaByte(void) { return loadedDisk->GetBiosType(); }
Bit8u fatDrive::GetMediaByte(void) {
return loadedDisk ? loadedDisk->GetBiosType() : 0;
}
// name can be a full DOS path with filename, up-to DOS_PATHLENGTH in length
bool fatDrive::FileCreate(DOS_File **file, char *name, Bit16u attributes) {
direntry fileEntry;
Bit32u dirClust, subEntry;
char dirName[DOS_NAMELENGTH_ASCII];
char pathName[11];
char pathName[11]; // pathName is actually just the filename, without path
Bit16u save_errorcode=dos.errorcode;
@ -1018,22 +1080,24 @@ bool fatDrive::FindFirst(char *_dir, DOS_DTA &dta,bool /*fcb_findfirst*/) {
return FindNextInternal(cwdDirCluster, dta, &dummyClust);
}
char* removeTrailingSpaces(char* str) {
char* end = str + strlen(str);
while((*--end == ' ') && (end > str)) {};
char* removeTrailingSpaces(char* str, const size_t max_len) {
char* end = str + strnlen(str, max_len);
while((*--end == ' ') && (end > str)) {
/* do nothing; break on while criteria */
}
*++end = '\0';
return str;
}
char* removeLeadingSpaces(char* str) {
size_t len = strlen(str);
size_t pos = strspn(str," ");
memmove(str,str + pos,len - pos + 1);
char* removeLeadingSpaces(char* str, const size_t max_len) {
const size_t len = strnlen(str, max_len);
const size_t pos = strspn(str, " ");
memmove(str, str + pos, len - pos + 1);
return str;
}
char* trimString(char* str) {
return removeTrailingSpaces(removeLeadingSpaces(str));
char* trimString(char* str, const size_t max_len) {
return removeTrailingSpaces(removeLeadingSpaces(str, max_len), max_len);
}
bool fatDrive::FindNextInternal(Bit32u dirClustNumber, DOS_DTA &dta, direntry *foundEntry) {
@ -1084,13 +1148,13 @@ nextfile:
memset(extension,0,4);
memcpy(find_name,&sectbuf[entryoffset].entryname[0],8);
memcpy(extension,&sectbuf[entryoffset].entryname[8],3);
trimString(&find_name[0]);
trimString(&extension[0]);
trimString(&find_name[0], sizeof(find_name));
trimString(&extension[0], sizeof(extension));
//if(!(sectbuf[entryoffset].attrib & DOS_ATTR_DIRECTORY))
if (extension[0]!=0) {
strcat(find_name, ".");
strcat(find_name, extension);
safe_strcat(find_name, ".");
safe_strcat(find_name, extension);
}
/* Compare attributes to search attributes */

View file

@ -20,8 +20,9 @@
#ifndef _DRIVES_H__
#define _DRIVES_H__
#include <vector>
#include <memory>
#include <string>
#include <vector>
#include <sys/types.h>
#include "dos_system.h"
#include "shell.h" /* for DOS_Shell */
@ -152,6 +153,8 @@ class imageDisk;
class fatDrive : public DOS_Drive {
public:
fatDrive(const char * sysFilename, Bit32u bytesector, Bit32u cylsector, Bit32u headscyl, Bit32u cylinders, Bit32u startSector);
fatDrive(const fatDrive&) = delete; // prevent copying
fatDrive& operator= (const fatDrive&) = delete; // prevent assignment
virtual bool FileOpen(DOS_File * * file,char * name,Bit32u flags);
virtual bool FileCreate(DOS_File * * file,char * name,Bit16u attributes);
virtual bool FileUnlink(char * name);
@ -182,11 +185,9 @@ public:
Bit32u getFirstFreeClust(void);
bool directoryBrowse(Bit32u dirClustNumber, direntry *useEntry, Bit32s entNum, Bit32s start=0);
bool directoryChange(Bit32u dirClustNumber, direntry *useEntry, Bit32s entNum);
imageDisk *loadedDisk;
std::unique_ptr<imageDisk> loadedDisk;
bool created_successfully;
private:
fatDrive(const fatDrive&); // prevent copying
fatDrive& operator= (const fatDrive&); // prevent assignment
Bit32u getClusterValue(Bit32u clustNum);
void setClusterValue(Bit32u clustNum, Bit32u clustValue);
Bit32u getClustFirstSect(Bit32u clustNum);