WitchLord said:
noizex said:
Then Angelscript would pick that up and it entered some sort of weird infinite loop for me - so this might be a candidate to reinforce against such mistakes, but you should be checking more strictly that you're passing actual byte stream and not garbage or 0.
Thanks for helping out and identifying the root cause.
AngelScript does sanity checks on the stream and normally would return an error (and write to the message stream the location in the stream where an error was identified), but obviously for this particular case AngelScript didn't detect the problem. I'll definitely look into this and add the necessary checks to properly handle this invalid input.
No problem, happy to help. After brief look at how Angelscript reads binary contents of that stream I think I know what's going on. Basically it starts allocating crazy amounts because it reads the size wrong:
count = ReadEncodedUInt();
module->enumTypes.Allocate(count, false);
This is first entry point into reading from the stream, and I checked - it ends up with enormous numbers being returned from ReadEncodedUInt() and I think I know why. Basically entire reading assumes that the stream can progress and will yield a sane data - for example the count for enum types in this case. But the way C++ streams work (as far as I could confirm) is that if the stream is invalid it will literally do nothing. So the flow is like this:
(from as_restore.cpp#1961)
asBYTE b; // !!uninitialized byte!
ReadData(&b, 1); // intends to read data from the stream into &b, but stream
// ignores it because we have a bad file, not writing to it
// and keeping the uninitialized value
bool isNegative = ( b & 0x80 ) ? true : false;
this gives b some arbitrary value and it continues its journey through the Read* method:
i = asUINT(b & 0x3F) << 8;
ReadData(&b, 1); i += b;
if( isNegative )
i = (asQWORD)(-asINT64(i));
how it progresses through many ifs will depend on what random value b assumed at the beginning (which was not overwritten by read from the stream), but it does end up with crazy values, my usual one is 18446744073709548468 which allocates this size for the enums buffer before it enters wild infinite loop and freezes my PC.
I think the first guard should be initializing b variable to something sensible like 0 because we can't trust the stream Read method that it will write anything to that pointer. Then maybe sanitization of data that's read from the stream - if we have some expectations of count/size it could validate it against the length of the stream and ensure that we're not reading some bizzare numbers from the moon and then try to allocate this amount of memory etc.
That's as far as I got - hope that's of any use to you @witchlord ?
PS. I think to reproduce it in most generic way, irrelevant of underlying file implementation used etc would be something like this:
class BorkenBytecodeStream : public asIBinaryStream
{
int Write(const void* ptr, asUINT size)
{
// may be implemented so we error on read not on write
}
int Read(void* ptr, asUINT size)
{
// noop, we don't write anything to ptr at all
return 0;
}
}
.png)
