Home > other >  BMP file read giving segmentation fault due to some invalid write of size 1
BMP file read giving segmentation fault due to some invalid write of size 1

Time：10-23

I am currently trying to read a BMP file and write it back out, and when I try and read the headers, it always ends up giving me a segfault. I have tried multiple different solutions from stack as well as other places and now am completely stuck.

Here's the BMPImage.h file:

#pragma once

#include <vector>
#include <cstdint>
#include <string>

#include "../fileHeaders/BMPHeaders.hpp"

#ifndef BMPIMAGE_H
#define BMPIMAGE_H


namespace mediaObjs {

    class BMPIMAGE {
        
        public:
            //Constructors
            BMPIMAGE(std::string filename);
            BMPIMAGE(std::string filename, BMPFILEHEADER fileHeader, 
                    BMPINFOHEADER infoHeader, RGBQUAD colourTable, std::vector<uint8_t> pixel_data);

            //IO methods
            void loadFile();
            void writeFile();

            //Getters
            std::string getFilename();
            BMPFILEHEADER getFileHeader();
            BMPINFOHEADER getInfoHeader();
            RGBQUAD getColourTable();
            std::vector<uint8_t> getPixelData();
            
            //Setters/Modifiers
            std::vector<uint8_t>* modifyPixelData();

        private:
            std::string m_filename;
            BMPFILEHEADER m_fileHeader;
            BMPINFOHEADER m_infoHeader;
            RGBQUAD m_colourTable;
            std::vector<uint8_t> m_pixel_data;


    };

}

#endif

Here's the BMPImage.cpp file:

#include "./BMPImage.h"

#include <fstream>
#include <algorithm>

using namespace mediaObjs;

BMPIMAGE::BMPIMAGE(std::string filename)
    : m_filename {filename}
{}

BMPIMAGE::BMPIMAGE(std::string filename, BMPFILEHEADER fileHeader, 
                    BMPINFOHEADER infoHeader, RGBQUAD colourTable, 
                    std::vector<uint8_t> pixel_data)
    : m_filename {filename}, m_fileHeader {fileHeader},
    m_infoHeader {infoHeader}, m_colourTable {colourTable},
    m_pixel_data {pixel_data}
{}

void BMPIMAGE::loadFile()
{

    std::ifstream file(this->m_filename, std::ios::in | std::ios::binary);

    //Read the file headers.
    BMPFILEHEADER fileHeader __attribute__((unused));
    file.read(reinterpret_cast<char*> (fileHeader.bfType), sizeof(uint16_t));
    file.read(reinterpret_cast<char*> (fileHeader.bfSize), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (fileHeader.bfReserved1), sizeof(uint16_t));
    file.read(reinterpret_cast<char*> (fileHeader.bfReserved2), sizeof(uint16_t));
    file.read(reinterpret_cast<char*> (fileHeader.bfOffBits), sizeof(uint32_t));
    this->m_fileHeader = fileHeader;

    //Read the info headers.
    BMPINFOHEADER infoHeader __attribute__((unused));
    file.read(reinterpret_cast<char*> (infoHeader.biSize), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biWidth), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biHeight), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biPlanes), sizeof(uint16_t));
    file.read(reinterpret_cast<char*> (infoHeader.biBitCount), sizeof(uint16_t));
    file.read(reinterpret_cast<char*> (infoHeader.biCompression), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biSizeImage), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biXPelsPerMetre), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biYPelsPerMetre), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biClrUsed), sizeof(uint32_t));
    file.read(reinterpret_cast<char*> (infoHeader.biClrImportant), sizeof(uint32_t));
    this->m_infoHeader = infoHeader;

    //Read the colour table.
    RGBQUAD colourTable __attribute__((unused));
    file.read(reinterpret_cast<char*> (colourTable.red), sizeof(uint8_t));
    file.read(reinterpret_cast<char*> (colourTable.green), sizeof(uint8_t));
    file.read(reinterpret_cast<char*> (colourTable.blue), sizeof(uint8_t));
    file.read(reinterpret_cast<char*> (colourTable.reserved), sizeof(uint8_t));
    this->m_colourTable = colourTable;

    //Read the pixel data.
    std::vector<char> buffer(m_infoHeader.biSizeImage);
    file.read(buffer.data(), m_infoHeader.biSizeImage);
    std::copy(std::begin(buffer), std::end(buffer), std::back_inserter(m_pixel_data));

    file.close();   
}

void BMPIMAGE::writeFile()
{

    std::ofstream file(this->m_filename, std::ios::out | std::ios::binary);

    //Write the file headers.
    file.write(reinterpret_cast<char*> (this->m_fileHeader.bfType), sizeof(uint16_t));
    file.write(reinterpret_cast<char*> (this->m_fileHeader.bfSize), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_fileHeader.bfReserved1), sizeof(uint16_t));
    file.write(reinterpret_cast<char*> (this->m_fileHeader.bfReserved2), sizeof(uint16_t));
    file.write(reinterpret_cast<char*> (this->m_fileHeader.bfOffBits), sizeof(uint32_t));

    //Write the info headers.
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biSize), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biWidth), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biHeight), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biPlanes), sizeof(uint16_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biBitCount), sizeof(uint16_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biCompression), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biSizeImage), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biXPelsPerMetre), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biYPelsPerMetre), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biClrUsed), sizeof(uint32_t));
    file.write(reinterpret_cast<char*> (this->m_infoHeader.biClrImportant), sizeof(uint32_t));

    //Write the colour table.
    file.write(reinterpret_cast<char*> (this->m_colourTable.red), sizeof(uint8_t));
    file.write(reinterpret_cast<char*> (this->m_colourTable.green), sizeof(uint8_t));
    file.write(reinterpret_cast<char*> (this->m_colourTable.blue), sizeof(uint8_t));
    file.write(reinterpret_cast<char*> (this->m_colourTable.reserved), sizeof(uint8_t));

    //Write the pixel data.
    uint8_t* pixel_data = &(this->m_pixel_data[0]);
    file.write(reinterpret_cast<char*> (pixel_data), this->m_pixel_data.size());

    file.close();

}

std::string BMPIMAGE::getFilename() 
{

    return this->m_filename;

}

BMPFILEHEADER BMPIMAGE::getFileHeader()
{

    return this->m_fileHeader;

}

BMPINFOHEADER BMPIMAGE::getInfoHeader()
{

    return this->m_infoHeader;

}

RGBQUAD BMPIMAGE::getColourTable()
{

    return this->m_colourTable;

}

std::vector<uint8_t> BMPIMAGE::getPixelData()
{

    return this->m_pixel_data;

}



std::vector<uint8_t>* BMPIMAGE::modifyPixelData()
{

    return &(this->m_pixel_data);

}

And this is the BMPHeaders.h file:

#pragma once

#include <cstdint>
#include <fstream>

#ifndef BMPHEADERS_H
#define BMPHEADERS_H


//Defines the BitMap File Header structure. Contains the information about the BitMap file.
struct __attribute__((__packed__)) BMPFILEHEADER
{

    uint16_t bfType;
    uint32_t bfSize;
    uint16_t bfReserved1;
    uint16_t bfReserved2;
    uint32_t bfOffBits; 

};

//Defines the BitMap Info Header structure. Contains the metadata about the actual BitMap image.
struct __attribute__((__packed__)) BMPINFOHEADER 
{

    uint32_t biSize;
    uint32_t biWidth;
    uint32_t biHeight;
    uint16_t biPlanes;
    uint16_t biBitCount;
    uint32_t biCompression;
    uint32_t biSizeImage;
    uint32_t biXPelsPerMetre;
    uint32_t biYPelsPerMetre;
    uint32_t biClrUsed;
    uint32_t biClrImportant;

};

//Defines the RGB structure for the BitMap file.
struct __attribute__((__packed__)) RGBQUAD 
{

    uint8_t red;
    uint8_t green;
    uint8_t blue;
    uint8_t reserved;

};


#endif

And the valgrind output dump:

==1488== Memcheck, a memory error detector
==1488== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1488== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==1488== Command: ./image-resizer
==1488==
==1488== error calling PR_SET_PTRACER, vgdb might block
Starting the deserialisation process.
Found the image file.
==1488== Use of uninitialised value of size 8
==1488==    at 0x4994D8E: std::basic_streambuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x495DC84: std::basic_filebuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x496BA81: std::istream::read(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x10AD01: mediaObjs::BMPIMAGE::loadFile() (BMPImage.cpp:26)
==1488==    by 0x10A5E6: main (main.cpp:23)
==1488==
==1488== Invalid write of size 1
==1488==    at 0x4994D8E: std::basic_streambuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x495DC84: std::basic_filebuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x496BA81: std::istream::read(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x10AD01: mediaObjs::BMPIMAGE::loadFile() (BMPImage.cpp:26)
==1488==    by 0x10A5E6: main (main.cpp:23)
==1488==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==1488==
==1488==
==1488== Process terminating with default action of signal 11 (SIGSEGV)
==1488==  Access not within mapped region at address 0x0
==1488==    at 0x4994D8E: std::basic_streambuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x495DC84: std::basic_filebuf<char, std::char_traits<char> >::xsgetn(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x496BA81: std::istream::read(char*, long) (in /usr/lib/x86_64-linux-gnu/libstdc  .so.6.0.28)
==1488==    by 0x10AD01: mediaObjs::BMPIMAGE::loadFile() (BMPImage.cpp:26)
==1488==    by 0x10A5E6: main (main.cpp:23)
==1488==  If you believe this happened as a result of a stack
==1488==  overflow in your program's main thread (unlikely but
==1488==  possible), you can try to increase the size of the
==1488==  main thread stack using the --main-stacksize= flag.
==1488==  The main thread stack size used in this run was 8388608.
==1488==
==1488== HEAP SUMMARY:
==1488==     in use at exit: 8,705 bytes in 3 blocks
==1488==   total heap usage: 6 allocs, 3 frees, 85,546 bytes allocated
==1488==
==1488== LEAK SUMMARY:
==1488==    definitely lost: 0 bytes in 0 blocks
==1488==    indirectly lost: 0 bytes in 0 blocks
==1488==      possibly lost: 0 bytes in 0 blocks
==1488==    still reachable: 8,705 bytes in 3 blocks
==1488==         suppressed: 0 bytes in 0 blocks
==1488== Reachable blocks (those to which a pointer was found) are not shown.
==1488== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==1488==
==1488== Use --track-origins=yes to see where uninitialised values come from
==1488== For lists of detected and suppressed errors, rerun with: -s
==1488== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
[1]    1488 segmentation fault (core dumped)  valgrind --leak-check=full ./image-resizer

Some help, or even a pointer in the right direction would really be appreciated.

EDIT: I have updated the code in accordance with @PaulMckenzie 's answer.

CodePudding user response：

reinterpret_cast<char*> (colourTable.red)

cast a value to a target pointer for read or write, this is nonsense, you should read or write to the address of value,

try :

reinterpret_cast<char*> (&colourTable.red)

and apply to all others similar value-to-pointer casts...

CodePudding user response：

The obvious errors are the following:

//Read the pixel data.
unsigned char* buffer = new unsigned char[this->m_infoHeader.biSizeImage];
//...
for(int i = 0; i <= sizeof(buffer);   i)

The first issue is that sizeof(buffer) does not give you the number of bytes allocated. It will return the sizeof(unsigned char*), which is more than likely 4 or 8.

The second issue is the usage of <= in the for loop. Even if you specified the correct number of bytes to process, there will be a memory overwrite on the last iteration of the for loop.

The quick fixes would be:

//Read the pixel data.
unsigned char* buffer = new unsigned char[this->m_infoHeader.biSizeImage];
file.read(reinterpret_cast<char*>(buffer), m_infoHeader.biSizeImage);
for(int i = 0; i < m_infoHeader.biSizeImage;   i){
    this->m_pixel_data.push_back(buffer[i]);
}

But a better (and safer) alternative would be:

//Read the pixel data.
#include <vector>
#include <iterator>
#include <algorithm>
//...
std::vector<char> buffer(m_infoHeader.biSizeImage);
file.read(buffer.data(), m_infoHeader.biSizeImage);
std::copy(std::begin(buffer), std::end(buffer), std::back_inserter(m_pixel_data));

Using std::vector, there is no need to issue a delete [] buffer; at the end of this processing. In addition, the std::copy replaces the hard-coded for loop that is issuing the push_back calls on the m_pixel_data vector.

Page link：https//www.codepudding.com/other/585038.html

Prev:Creating a vector of the type of std::any
Next:Does std::string class handle clean up if the assignment operator fails due to length_error or bad_a
  • Related

About Us:  Contact Us      Terms of Service       Privacy Policy

Copyright © 2010-2023，Powered By CodePudding