Opening a project with a large library crashes: short read feeds the unsafe LZ4 decoder in LargeListMessagePack

[pedronobrez]

Summary

LargeListMessagePack.FillFromStream issues a single Stream.Read and treats a short read as a
complete one. The partly-filled buffer is then handed to MessagePack's unsafe LZ4 decoder with
the full expected length, so the decoder reads past the valid data. On a .mdproject whose
database entry is large this kills the process with an AccessViolationException, which no
try/catch can stop.

The symptom is: opening a saved project that used a large library crashes MS-DIAL outright.

Where

src/Common/CommonStandard/MessagePack/LargeListMessagePack.cs

static bool FillFromStream(Stream input, ref byte[] buffer, int offset, int readSize)
{
    int length = 0;
    int read;
    if ((read = input.Read(buffer, offset, readSize)) > 0)   // <-- one Read, no loop
    {
        length += read;
        if (length == buffer.Length)
        {
            MessagePackBinary.FastResize(ref buffer, length * 2);
        }
        return true;
    }
    return false;
}

Stream.Read is only required to return at least one byte; it may return fewer than readSize.
The DeflateStream inside a ZipArchive — which is how ProjectDataStorage / DataBaseItem<T>
store a database inside a .mdproject — routinely returns short reads on a large entry.

DeserializeEach then does:

if (FillFromStream(stream, ref buffer, 0, bufferLength))
{
    bytes = new ArraySegment<byte>(buffer, 0, bufferLength);   // claims bufferLength valid bytes
    ...
    LZ4Codec.Decode(bytes.Array, bytes.Offset, len, bufferLz4, 0, length);
}

LZ4Codec.Decode goes to LZ4_uncompress_64, which walks the input with raw pointers, so reading
past the end of the valid region is an access violation rather than an exception.

Stack trace

Fatal error. System.AccessViolationException: Attempted to read or write protected memory.
   at MessagePack.LZ4.LZ4Codec.LZ4_uncompress_64(Byte*, Byte*, Int32)
   at MessagePack.LZ4.LZ4Codec.Decode64Unsafe(Byte[], Int32, Int32, Byte[], Int32, Int32)
   at MessagePack.LZ4.LZ4Codec.Decode(Byte[], Int32, Int32, Byte[], Int32, Int32)
   at CompMs.Common.MessagePack.LargeListMessagePack.DeserializeEach[...](Stream, Byte[], IFormatterResolver)
   at CompMs.Common.MessagePack.LargeListMessagePack.DeserializeCore[...](Stream, IFormatterResolver)
   at CompMs.Common.MessagePack.LargeListMessagePack.Deserialize[...](Stream, IFormatterResolver)
   at CompMs.MsdialCore.DataObj.MoleculeDataBase.Load(Stream, String)
   at CompMs.MsdialCore.DataObj.DataBaseItem`1.TryCurrentLoad(ZipArchive, String, ILoadAnnotatorVisitor, IAnnotationQueryFactoryGenerationVisitor, String)
   at CompMs.MsdialLcMsApi.DataObj.MsdialLcmsDataStorage+MsdialLcmsSerializer.LoadDataBasesAsync(...)
   at CompMs.MsdialCore.DataObj.MsdialDataStorageBase+MsdialSerializer.LoadAsync(...)
   at CompMs.MsdialIntegrate.Parser.MsdialIntegrateSerializer.LoadAsync(...)
   at CompMs.MsdialCore.DataObj.ProjectDataStorage.LoadDataStorage(...)
   at CompMs.MsdialCore.DataObj.ProjectDataStorage.LoadAsync(...)

Reproduction

1. Process an LC-MS dataset with a large MSP library (mine has 449 627 records, ~224 MB as text,
   ~52 MB as the serialised _Loaded.msp2.dbs) and save the project.
2. Open the saved .mdproject.

It does not fail every time, because whether a single Read happens to return the whole entry
depends on where the zip's chunk boundaries fall. In my case one project opened and another,
processed from the same data with the same library, crashed reproducibly.

A deterministic reproduction without any data: serialise a list large enough to exceed one 65536
buffer with LargeListMessagePack.Serialize, then deserialise it through a Stream whose Read
returns at most a few bytes per call.

Suggested fix

Read to completion:

static bool FillFromStream(Stream input, ref byte[] buffer, int offset, int readSize)
{
    int length = 0;
    while (length < readSize)
    {
        var read = input.Read(buffer, offset + length, readSize - length);
        if (read <= 0) break;
        length += read;
    }
    if (length <= 0)
    {
        return false;
    }
    if (length == buffer.Length)
    {
        MessagePackBinary.FastResize(ref buffer, length * 2);
    }
    return true;
}

DeserializeCore, DeserializeIncrementalCore, DeserializeEach and DeserializeAt all go
through this method, so the one change covers every caller.

I have been running this fix against MS-DIAL 5.5.260817. The project that used to kill the process
now opens, and its database loads with all 449 627 records. I also added a regression test that
round-trips a payload larger than one buffer through a stream returning 1, 17, 4096 and 65536 bytes
per read; it fails without the change and passes with it.

Happy to open a pull request if that is useful.

Context

Found while building an open, cross-platform port of MS-DIAL 5 for macOS and Linux, which keeps the
upstream sources as close to unmodified as possible. The bug is in shared code and is not specific
to that port — it should affect the Windows application the same way.