- use FileRdr in the PNG code, in particular to sanitize the savepic handling.

This commit is contained in:
Christoph Oelckers 2018-03-10 19:22:26 +01:00
parent 5fa63c396d
commit c4387459bb
6 changed files with 84 additions and 95 deletions

View file

@ -545,6 +545,11 @@ public:
Close();
}
bool isOpen() const
{
return mReader != nullptr;
}
void Close()
{
if (mReader != nullptr) delete mReader;

View file

@ -75,22 +75,17 @@ struct IHDR
uint8_t Interlace;
};
PNGHandle::PNGHandle (FILE *file) : File(0), bDeleteFilePtr(true), ChunkPt(0)
PNGHandle::PNGHandle (FileRdr &file) : File(0), bDeleteFilePtr(true), ChunkPt(0)
{
File = new FileReader(file);
File = std::move(file);
}
PNGHandle::PNGHandle (FileReader *file, bool takereader) : File(file), bDeleteFilePtr(takereader), ChunkPt(0) {}
PNGHandle::~PNGHandle ()
{
for (unsigned int i = 0; i < TextChunks.Size(); ++i)
{
delete[] TextChunks[i];
}
if (bDeleteFilePtr)
{
delete File;
}
}
// EXTERNAL FUNCTION PROTOTYPES --------------------------------------------
@ -309,7 +304,7 @@ unsigned int M_NextPNGChunk (PNGHandle *png, uint32_t id)
{
if (png->Chunks[png->ChunkPt].ID == id)
{ // Found the chunk
png->File->Seek (png->Chunks[png->ChunkPt++].Offset, SEEK_SET);
png->File.Seek (png->Chunks[png->ChunkPt++].Offset, FileRdr::SeekSet);
return png->Chunks[png->ChunkPt - 1].Size;
}
}
@ -374,46 +369,43 @@ bool M_GetPNGText (PNGHandle *png, const char *keyword, char *buffer, size_t buf
//
//==========================================================================
PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader)
PNGHandle *M_VerifyPNG (FileRdr &filer)
{
PNGHandle::Chunk chunk;
PNGHandle *png;
uint32_t data[2];
bool sawIDAT = false;
if (filer->Read(&data, 8) != 8)
if (filer.Read(&data, 8) != 8)
{
if (takereader) delete filer;
return NULL;
}
if (data[0] != MAKE_ID(137,'P','N','G') || data[1] != MAKE_ID(13,10,26,10))
{ // Does not have PNG signature
if (takereader) delete filer;
return NULL;
}
if (filer->Read (&data, 8) != 8)
if (filer.Read (&data, 8) != 8)
{
if (takereader) delete filer;
return NULL;
}
if (data[1] != MAKE_ID('I','H','D','R'))
{ // IHDR must be the first chunk
if (takereader) delete filer;
return NULL;
}
// It looks like a PNG so far, so start creating a PNGHandle for it
png = new PNGHandle (filer, takereader);
png = new PNGHandle (filer);
// filer is no longer valid after the above line!
chunk.ID = data[1];
chunk.Offset = 16;
chunk.Size = BigLong((unsigned int)data[0]);
png->Chunks.Push (chunk);
filer->Seek (16, SEEK_SET);
png->File.Seek (16, FileRdr::SeekSet);
while (filer->Seek (chunk.Size + 4, SEEK_CUR) == 0)
while (png->File.Seek (chunk.Size + 4, FileRdr::SeekCur) == 0)
{
// If the file ended before an IEND was encountered, it's not a PNG.
if (filer->Read (&data, 8) != 8)
if (png->File.Read (&data, 8) != 8)
{
break;
}
@ -432,7 +424,7 @@ PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader)
sawIDAT = true;
}
chunk.ID = data[1];
chunk.Offset = filer->Tell();
chunk.Offset = (uint32_t)png->File.Tell();
chunk.Size = BigLong((unsigned int)data[0]);
png->Chunks.Push (chunk);
@ -441,7 +433,7 @@ PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader)
{
char *str = new char[chunk.Size + 1];
if (filer->Read (str, chunk.Size) != (long)chunk.Size)
if (png->File.Read (str, chunk.Size) != chunk.Size)
{
delete[] str;
break;
@ -452,6 +444,7 @@ PNGHandle *M_VerifyPNG (FileReader *filer, bool takereader)
}
}
filer = std::move(png->File); // need to get the reader back if this function failed.
delete png;
return NULL;
}
@ -477,7 +470,7 @@ void M_FreePNG (PNGHandle *png)
//
//==========================================================================
bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int pitch,
bool M_ReadIDAT (FileRdr &file, uint8_t *buffer, int width, int height, int pitch,
uint8_t bitdepth, uint8_t colortype, uint8_t interlace, unsigned int chunklen)
{
// Uninterlaced images are treated as a conceptual eighth pass by these tables.
@ -574,7 +567,7 @@ bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int p
if (stream.avail_in == 0 && chunklen > 0)
{
stream.next_in = chunkbuffer;
stream.avail_in = (uInt)file->Read (chunkbuffer, MIN<uint32_t>(chunklen,sizeof(chunkbuffer)));
stream.avail_in = (uInt)file.Read (chunkbuffer, MIN<uint32_t>(chunklen,sizeof(chunkbuffer)));
chunklen -= stream.avail_in;
}
@ -666,7 +659,7 @@ bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int p
{
uint32_t x[3];
if (file->Read (x, 12) != 12)
if (file.Read (x, 12) != 12)
{
lastIDAT = true;
}

View file

@ -36,8 +36,8 @@
#include <stdio.h>
#include "doomtype.h"
#include "v_video.h"
#include "files.h"
class FileReader;
class FileWriter;
// PNG Writing --------------------------------------------------------------
@ -64,7 +64,6 @@ bool M_SaveBitmap(const uint8_t *from, ESSType color_type, int width, int height
// PNG Reading --------------------------------------------------------------
class FileReader;
struct PNGHandle
{
struct Chunk
@ -74,14 +73,13 @@ struct PNGHandle
uint32_t Size;
};
FileReader *File;
FileRdr File;
bool bDeleteFilePtr;
TArray<Chunk> Chunks;
TArray<char *> TextChunks;
unsigned int ChunkPt;
PNGHandle(FILE *file);
PNGHandle(FileReader *file, bool takereader = false);
PNGHandle(FileRdr &file);
~PNGHandle();
};
@ -89,7 +87,7 @@ struct PNGHandle
// the signature, but also checking for the IEND chunk. CRC checking of
// each chunk is not done. If it is valid, you get a PNGHandle to pass to
// the following functions.
PNGHandle *M_VerifyPNG (FileReader *file, bool takereader = false);
PNGHandle *M_VerifyPNG (FileRdr &file);
// Finds a chunk in a PNG file. The file pointer will be positioned at the
// beginning of the chunk data, and its length will be returned. A return
@ -108,7 +106,7 @@ bool M_GetPNGText (PNGHandle *png, const char *keyword, char *buffer, size_t buf
// The file must be positioned at the start of the first IDAT. It reads
// image data into the provided buffer. Returns true on success.
bool M_ReadIDAT (FileReader *file, uint8_t *buffer, int width, int height, int pitch,
bool M_ReadIDAT (FileRdr &file, uint8_t *buffer, int width, int height, int pitch,
uint8_t bitdepth, uint8_t colortype, uint8_t interlace, unsigned int idatlen);

View file

@ -236,8 +236,8 @@ void FSavegameManager::ReadSaveStrings()
}
else // check for old formats.
{
FileReader file;
if (file.Open(filepath))
FileRdr file;
if (file.OpenFile(filepath))
{
PNGHandle *png;
char sig[16];
@ -255,7 +255,7 @@ void FSavegameManager::ReadSaveStrings()
title[OLDSAVESTRINGSIZE] = 0;
if (nullptr != (png = M_VerifyPNG(&file, false)))
if (nullptr != (png = M_VerifyPNG(file)))
{
char *ver = M_GetPNGText(png, "ZDoom Save Version");
if (ver != nullptr)
@ -272,7 +272,7 @@ void FSavegameManager::ReadSaveStrings()
}
else
{
file.Seek(0, SEEK_SET);
file.Seek(0, FileRdr::SeekSet);
if (file.Read(sig, 16) == 16)
{
@ -496,33 +496,30 @@ unsigned FSavegameManager::ExtractSaveData(int index)
FResourceLump *pic = resf->FindLump("savepic.png");
if (pic != nullptr)
{
FileReader *reader = pic->NewReader();
if (reader != nullptr)
FileRdr picreader;
picreader.OpenMemoryArray([=](TArray<uint8_t> &array)
{
// copy to a memory buffer which gets accessed through a memory reader and PNGHandle.
// We cannot use the actual lump as backing for the texture because that requires keeping the
// savegame file open.
SavePicData.Resize(pic->LumpSize);
reader->Read(&SavePicData[0], pic->LumpSize);
reader = new MemoryReader(&SavePicData[0], SavePicData.Size());
PNGHandle *png = M_VerifyPNG(reader);
auto cache = pic->CacheLump();
array.Resize(pic->LumpSize);
memcpy(&array[0], cache, pic->LumpSize);
pic->ReleaseCache();
return true;
});
PNGHandle *png = M_VerifyPNG(picreader);
if (png != nullptr)
{
SavePic = PNGTexture_CreateFromFile(png, node->Filename);
currentSavePic = reader; // must be kept so that the texture can read from it.
delete png;
if (SavePic->GetWidth() == 1 && SavePic->GetHeight() == 1)
{
delete SavePic;
SavePic = nullptr;
delete currentSavePic;
currentSavePic = nullptr;
SavePicData.Clear();
}
}
}
}
}
delete resf;
}
return index;
@ -551,14 +548,9 @@ void FSavegameManager::UnloadSaveData()
{
V_FreeBrokenLines(SaveComment);
}
if (currentSavePic != nullptr)
{
delete currentSavePic;
}
SavePic = nullptr;
SaveComment = nullptr;
currentSavePic = nullptr;
SavePicData.Clear();
}

View file

@ -71,7 +71,6 @@ private:
FSaveGameNode NewSaveNode;
int LastSaved = -1;
int LastAccessed = -1;
FileReader *currentSavePic = nullptr;
TArray<char> SavePicData;
FTexture *SavePic = nullptr;
FBrokenLines *SaveComment = nullptr;

View file

@ -51,7 +51,7 @@
class FPNGTexture : public FTexture
{
public:
FPNGTexture (FileReader &lump, int lumpnum, const FString &filename, int width, int height, uint8_t bitdepth, uint8_t colortype, uint8_t interlace);
FPNGTexture (FileRdr &lump, int lumpnum, const FString &filename, int width, int height, uint8_t bitdepth, uint8_t colortype, uint8_t interlace);
~FPNGTexture ();
const uint8_t *GetColumn (unsigned int column, const Span **spans_out);
@ -66,7 +66,7 @@ protected:
FString SourceFile;
uint8_t *Pixels;
Span **Spans;
FileReader *fr;
FileRdr fr;
uint8_t BitDepth;
uint8_t ColorType;
@ -147,7 +147,7 @@ FTexture *PNGTexture_TryCreate(FileRdr & data, int lumpnum)
}
}
return new FPNGTexture (*data.Reader(), lumpnum, FString(), BigLong((int)width), BigLong((int)height),
return new FPNGTexture (data, lumpnum, FString(), BigLong((int)width), BigLong((int)height),
bitdepth, colortype, interlace);
}
@ -168,9 +168,9 @@ FTexture *PNGTexture_CreateFromFile(PNGHandle *png, const FString &filename)
}
// Check the IHDR to make sure it's a type of PNG we support.
png->File->Read(&width, 4);
png->File->Read(&height, 4);
(*png->File) >> bitdepth >> colortype >> compression >> filter >> interlace;
png->File.Read(&width, 4);
png->File.Read(&height, 4);
png->File >> bitdepth >> colortype >> compression >> filter >> interlace;
if (compression != 0 || filter != 0 || interlace > 1)
{
@ -185,7 +185,7 @@ FTexture *PNGTexture_CreateFromFile(PNGHandle *png, const FString &filename)
return NULL;
}
return new FPNGTexture (*png->File, -1, filename, BigLong((int)width), BigLong((int)height),
return new FPNGTexture (png->File, -1, filename, BigLong((int)width), BigLong((int)height),
bitdepth, colortype, interlace);
}
@ -195,7 +195,7 @@ FTexture *PNGTexture_CreateFromFile(PNGHandle *png, const FString &filename)
//
//==========================================================================
FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename, int width, int height,
FPNGTexture::FPNGTexture (FileRdr &lump, int lumpnum, const FString &filename, int width, int height,
uint8_t depth, uint8_t colortype, uint8_t interlace)
: FTexture(NULL, lumpnum), SourceFile(filename), Pixels(0), Spans(0),
BitDepth(depth), ColorType(colortype), Interlace(interlace), HaveTrans(false),
@ -210,9 +210,6 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename
uint32_t len, id;
int i;
if (lumpnum == -1) fr = &lump;
else fr = nullptr;
UseType = TEX_MiscPatch;
LeftOffset = 0;
TopOffset = 0;
@ -225,7 +222,7 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename
memset(trans, 255, 256);
// Parse pre-IDAT chunks. I skip the CRCs. Is that bad?
lump.Seek(33, SEEK_SET);
lump.Seek(33, FileRdr::SeekSet);
lump.Read(&len, 4);
lump.Read(&id, 4);
@ -235,7 +232,7 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename
switch (id)
{
default:
lump.Seek (len, SEEK_CUR);
lump.Seek (len, FileRdr::SeekCur);
break;
case MAKE_ID('g','r','A','b'):
@ -268,7 +265,7 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename
lump.Read (p.pngpal, PaletteSize * 3);
if (PaletteSize * 3 != (int)len)
{
lump.Seek (len - PaletteSize * 3, SEEK_CUR);
lump.Seek (len - PaletteSize * 3, FileRdr::SeekCur);
}
for (i = PaletteSize - 1; i >= 0; --i)
{
@ -290,12 +287,12 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename
bMasked = true;
break;
}
lump.Seek(4, SEEK_CUR); // Skip CRC
lump.Seek(4, FileRdr::SeekCur); // Skip CRC
lump.Read(&len, 4);
id = MAKE_ID('I','E','N','D');
lump.Read(&id, 4);
}
StartOfIDAT = lump.Tell() - 8;
StartOfIDAT = (uint32_t)lump.Tell() - 8;
switch (colortype)
{
@ -342,6 +339,8 @@ FPNGTexture::FPNGTexture (FileReader &lump, int lumpnum, const FString &filename
bMasked = HaveTrans;
break;
}
// If this is a savegame we must keep the reader.
if (lumpnum == -1) fr = std::move(lump);
}
//==========================================================================
@ -457,15 +456,17 @@ const uint8_t *FPNGTexture::GetPixels ()
void FPNGTexture::MakeTexture ()
{
FileReader *lump;
FileRdr *lump;
FileRdr lfr;
if (SourceLump >= 0)
{
lump = new FWadLump(Wads.OpenLumpNum(SourceLump));
lfr = Wads.OpenLumpReader(SourceLump);
lump = &lfr;
}
else
{
lump = fr;// new FileReader(SourceFile.GetChars());
lump = &fr;
}
Pixels = new uint8_t[Width*Height];
@ -476,13 +477,13 @@ void FPNGTexture::MakeTexture ()
else
{
uint32_t len, id;
lump->Seek (StartOfIDAT, SEEK_SET);
lump->Seek (StartOfIDAT, FileRdr::SeekSet);
lump->Read(&len, 4);
lump->Read(&id, 4);
if (ColorType == 0 || ColorType == 3) /* Grayscale and paletted */
{
M_ReadIDAT (lump, Pixels, Width, Height, Width, BitDepth, ColorType, Interlace, BigLong((unsigned int)len));
M_ReadIDAT (*lump, Pixels, Width, Height, Width, BitDepth, ColorType, Interlace, BigLong((unsigned int)len));
if (Width == Height)
{
@ -518,7 +519,7 @@ void FPNGTexture::MakeTexture ()
uint8_t *in, *out;
int x, y, pitch, backstep;
M_ReadIDAT (lump, tempix, Width, Height, Width*bytesPerPixel, BitDepth, ColorType, Interlace, BigLong((unsigned int)len));
M_ReadIDAT (*lump, tempix, Width, Height, Width*bytesPerPixel, BitDepth, ColorType, Interlace, BigLong((unsigned int)len));
in = tempix;
out = Pixels;
@ -602,7 +603,6 @@ void FPNGTexture::MakeTexture ()
delete[] tempix;
}
}
if (lump != fr) delete lump;
}
//===========================================================================
@ -616,21 +616,24 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo
// Parse pre-IDAT chunks. I skip the CRCs. Is that bad?
PalEntry pe[256];
uint32_t len, id;
FileReader *lump;
static char bpp[] = {1, 0, 3, 1, 2, 0, 4};
int pixwidth = Width * bpp[ColorType];
int transpal = false;
FileRdr *lump;
FileRdr lfr;
if (SourceLump >= 0)
{
lump = new FWadLump(Wads.OpenLumpNum(SourceLump));
lfr = Wads.OpenLumpReader(SourceLump);
lump = &lfr;
}
else
{
lump = fr;// new FileReader(SourceFile.GetChars());
lump = &fr;
}
lump->Seek(33, SEEK_SET);
lump->Seek(33, FileRdr::SeekSet);
for(int i = 0; i < 256; i++) // default to a gray map
pe[i] = PalEntry(255,i,i,i);
@ -642,7 +645,7 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo
switch (id)
{
default:
lump->Seek (len, SEEK_CUR);
lump->Seek (len, FileRdr::SeekCur);
break;
case MAKE_ID('P','L','T','E'):
@ -664,11 +667,11 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo
}
else
{
lump->Seek(len, SEEK_CUR);
lump->Seek(len, FileRdr::SeekCur);
}
break;
}
lump->Seek(4, SEEK_CUR); // Skip CRC
lump->Seek(4, FileRdr::SeekCur); // Skip CRC
lump->Read(&len, 4);
id = MAKE_ID('I','E','N','D');
lump->Read(&id, 4);
@ -682,11 +685,10 @@ int FPNGTexture::CopyTrueColorPixels(FBitmap *bmp, int x, int y, int rotate, FCo
uint8_t * Pixels = new uint8_t[pixwidth * Height];
lump->Seek (StartOfIDAT, SEEK_SET);
lump->Seek (StartOfIDAT, FileRdr::SeekSet);
lump->Read(&len, 4);
lump->Read(&id, 4);
M_ReadIDAT (lump, Pixels, Width, Height, pixwidth, BitDepth, ColorType, Interlace, BigLong((unsigned int)len));
if (lump != fr) delete lump;
M_ReadIDAT (*lump, Pixels, Width, Height, pixwidth, BitDepth, ColorType, Interlace, BigLong((unsigned int)len));
switch (ColorType)
{