Should RelocationsDirectory::rebuild add an end mark?

[MoePus]

// RelocationsDirectory.cpp : 83
void RelocationsDirectory::rebuild(std::vector<byte>& vBuffer) const
{
    OutputBuffer obBuffer(vBuffer);

    for (unsigned int i=0;i<m_vRelocations.size();i++)
    {
        obBuffer << m_vRelocations[i].ibrRelocation.VirtualAddress;
        obBuffer << m_vRelocations[i].ibrRelocation.SizeOfBlock;

        for (unsigned int j=0;j<m_vRelocations[i].vRelocData.size();j++)
        {
            obBuffer << m_vRelocations[i].vRelocData[j];
        }
    }
    obBuffer << (unsigned long long)0; // add this line
}

[s3rvac]

Hi. Could you please tell us why should the following line be there?

obBuffer << (unsigned long long)0; // add this line

The rebuild() member function is used in the following context:

std::vector<unsigned char> vBuffer;
rebuild(vBuffer);
ofFile.write(reinterpret_cast<const char*>(vBuffer.data()), static_cast<std::streamsize>(vBuffer.size()));

This writes exactly the number of bytes that are stored in vBuffer (notice that the length of the buffer is passed via the second parameter of ofFile.write()). Adding trailing zeros would write them to the output as the size of the buffer would increase.

In the PE format, there are both PointerToRelocations and NumberOfRelocations, so I do not see why we should end them with null bytes. Can you please elaborate?

[MoePus]

yes. i have added a reloc group and removed lots of relocs of original PE. my code

    auto& relocDir = file.relocDir();
    auto nReloc = relocDir.calcNumberOfRelocations();

    // ...remove original relocs
    // add my own reloc
    relocDir.addRelocation();
    relocDir.setVirtualAddress(nReloc, myRVA);
    relocDir.addRelocationData(nReloc, (3 << 12)); // low offset is zero
    relocDir.setSizeOfBlock(nReloc, 2 * 1 + 8);

    relocDir.write(outfilePath, header.rvaToOffset(header.getIddBaseRelocRva()));

without an end mark note there is 48 reloc groups.and the last one is wrong.

with the mark

the RelocTable is end with 4 byte zero see 010editor EXE.bt

typedef struct
{   
    local ULONG ulRelocNum=0;

    while(1)
    {
        if(0 == ReadUInt(FTell()) )
        {
            break;
        }
        IMAGE_BASE_RELOCATION BaseReloc;
        ulRelocNum++;
    }
}BASE_RELOCATION_TABLE <comment=commentBaseRelocationTable>;

[s3rvac]

@ladislav-zezula Could you please take a look at this?

[ladislav-zezula]

After careful review, we decided to remove the function completely. In retdec, we only use pelib for read-only operations and rebuilding relocations is more work than just calling one function.

PR: https://github.com/avast/pelib/pull/16

Thank you for bringing this to attention.

[s3rvac]

I have merged #16, so I am closing the issue.