Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack corruption in EbmlElement.cpp MakeRenderHead() #224

Open
poikosoft opened this issue Jan 17, 2024 · 3 comments
Open

Stack corruption in EbmlElement.cpp MakeRenderHead() #224

poikosoft opened this issue Jan 17, 2024 · 3 comments
Labels

Comments

@poikosoft
Copy link

I got stack corruption in this function because the binary buffer was not large enough

64-bit coded size should be 12 octets ?

See the code:

filepos_t EbmlElement::MakeRenderHead(IOCallback & output, bool bKeepPosition)
{
//std::array<binary, 4 + 8> FinalHead; // Class D + 64 bits coded size
std::array<binary, 4 + 12> FinalHead; // Class D + 64 bits coded size (=12?)
std::size_t FinalHeadSize;

FinalHeadSize = EBML_ID_LENGTH((const EbmlId&)*this);
EbmlId(*this).Fill(FinalHead.data());

const int CodedSize = CodedSizeLength(Size, SizeLength, bSizeIsFinite);
CodedValueLength(Size, CodedSize, &FinalHead.at(FinalHeadSize));
FinalHeadSize += CodedSize;

output.writeFully(FinalHead.data(), FinalHeadSize);
if (!bKeepPosition) {
ElementPosition = output.getFilePointer() - FinalHeadSize;
SizePosition = ElementPosition + EBML_ID_LENGTH((const EbmlId&)*this);
}

return FinalHeadSize;
}

@robUx4
Copy link
Contributor

robUx4 commented Jan 21, 2024

It seems that you are using the master version rather than the stable build. It shouldn't change much with this issue, but you may try the stable version.

Do you know what values caused this ? A stacktrace with the values in use could help.

We do not support writing EBML ID's bigger than 4 bytes and length values bigger than 8 bytes (64-bit integer). If you have an EBML ID with a size of 5 bytes this won't work, and other EBML code will likely not be able to read it. As for the data size, it's used as a 64-bit integer but only 56 bits are actually usable in EBML: https://www.rfc-editor.org/rfc/rfc8794.html#section-6.3. So if your data is extremly large (or the value is bogus) it can't be stored either. That can also be the cause of your issue.

In both case I assume bogus data, which the library doesn't check (it's not really designed for that).

@robUx4 robUx4 added the bug label Jan 21, 2024
@poikosoft
Copy link
Author

It may have been that I wrote a too large value by accident (bigger value than 2^56) to a Matroska SeekHead / Seek / Position

I just tested again now that I have fully working tag editing code (that also updates the SeekHead). The error no longer happens.

So I guess it was my fault, but the library could throw an error (or debug assert) rather than corrupting the stack.

@mbunkus
Copy link
Contributor

mbunkus commented Jan 24, 2024

The library should definitely handle semantically invalid input gracefully, not by invalid memory access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants