Matroska-Org / libmatroska

a C++ libary to parse Matroska files (.mkv and .mka)
GNU Lesser General Public License v2.1
313 stars 57 forks source link

Matroska version checking is bogus #164

Closed robUx4 closed 6 months ago

robUx4 commented 6 months ago

The verification checks don't work with this simple code:

  KaxClusterPosition t;
  const auto & spec = t.ElementSpec();
  const auto profiles = reinterpret_cast<const MatroskaProfile *>(&spec.GetVersions());
  assert(profiles->IsValidInVersion(1));

The pointer from GetVersions is bogus (also fails when not using pointers). It might be an issue with the storage in EbmlCallbacks which is not big enough. We need unit tests in libebml and libmatroska to make sure it's OK and remains that way.

robUx4 commented 6 months ago

It works if we store the EbmlDocVersion as a pointer and have an actual instance of the version kept in memory. Maybe we can store it as a reference but we can't initialize it the ways we do not. Or maybe it needs a copy constructor in EbmlDocVersion and MatroskaProfile ?

robUx4 commented 6 months ago

The code above doesn't work because the MatroskaProfile passed to the constructor is a temporary value. The object is destroyed after the initialization (in the static + global variables). The references point to a dead/reused memory location.

Example with this:

class A {
public:
    constexpr A(bool i) : toto(i?1:0) {}
    const int toto;
};

class B : public A {
public:
    constexpr B(bool o, bool i) : A(o), titi(i) {}
    const bool titi;
};

class holder {
public:
    constexpr holder(const A & ref)
    :myref(ref)
    {}

    const A & myref;
};

const holder holder_local(B(true, false));
constexpr const holder holder_local_expr(B{true, false});

The second constructor fails to compile because the constructor is a temporary value.

So we need to make the MatroskaProfile passed to the EbmlCallbacks constructor actual object instances kept in memory. If we can manage to turn the global EbmlCallbacks instances constexpr, it should generate build failure when not used properly, as in the example above.