Skip to content

Commit 193091f

Browse files
authored
Fix unbounded recursion in EBML/Matroska MasterElement and MP4 atoms (#1326)
Credits for fix and reporting go to https://github.com/ericliu-12.
1 parent 5d63187 commit 193091f

4 files changed

Lines changed: 37 additions & 5 deletions

File tree

taglib/matroska/ebml/ebmlmasterelement.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "ebmlmasterelement.h"
2222
#include "ebmlvoidelement.h"
2323
#include "ebmlutils.h"
24+
#include "tdebug.h"
2425
#include "tfile.h"
2526

2627
using namespace TagLib;
@@ -97,18 +98,34 @@ void EBML::MasterElement::setMinRenderSize(offset_t minimumSize)
9798
minRenderSize = minimumSize;
9899
}
99100

100-
bool EBML::MasterElement::read(File &file)
101+
bool EBML::MasterElement::read(File &file, int depth)
101102
{
103+
static constexpr int MAX_EBML_DEPTH = 64;
104+
if(depth > MAX_EBML_DEPTH) {
105+
debug("EBML: Maximum nesting depth exceeded");
106+
return false;
107+
}
102108
const offset_t maxOffset = file.tell() + dataSize;
103109
std::unique_ptr<Element> element;
104110
while((element = findNextElement(file, maxOffset))) {
105-
if(!element->read(file))
106-
return false;
111+
if(auto master = dynamic_cast<MasterElement *>(element.get())) {
112+
if(!master->read(file, depth + 1))
113+
return false;
114+
}
115+
else {
116+
if(!element->read(file))
117+
return false;
118+
}
107119
elements.push_back(std::move(element));
108120
}
109121
return file.tell() == maxOffset;
110122
}
111123

124+
bool EBML::MasterElement::read(File &file)
125+
{
126+
return read(file, 0);
127+
}
128+
112129
ByteVector EBML::MasterElement::render()
113130
{
114131
ByteVector buffer = renderId();

taglib/matroska/ebml/ebmlmasterelement.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ namespace TagLib
5555
void setMinRenderSize(offset_t minimumSize);
5656

5757
protected:
58+
bool read(File &file, int depth);
59+
5860
offset_t offset;
5961
offset_t padding = 0;
6062
offset_t minRenderSize = 0;

taglib/mp4/mp4atom.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class MP4::Atom::AtomPrivate
5151
AtomList children;
5252
};
5353

54-
MP4::Atom::Atom(File *file)
54+
MP4::Atom::Atom(File *file, int depth)
5555
: d(std::make_unique<AtomPrivate>(file->tell()))
5656
{
5757
d->children.setAutoDelete(true);
@@ -109,8 +109,13 @@ MP4::Atom::Atom(File *file)
109109
else if(d->name == "stsd") {
110110
file->seek(8, File::Current);
111111
}
112+
static constexpr int MAX_MP4_ATOM_DEPTH = 64;
113+
if(depth > MAX_MP4_ATOM_DEPTH) {
114+
debug("MP4: Maximum nesting depth exceeded");
115+
return;
116+
}
112117
while(file->tell() < d->offset + d->length) {
113-
auto child = new MP4::Atom(file);
118+
auto child = new MP4::Atom(file, depth + 1);
114119
d->children.append(child);
115120
if(child->d->length == 0)
116121
return;
@@ -122,6 +127,11 @@ MP4::Atom::Atom(File *file)
122127
file->seek(d->offset + d->length);
123128
}
124129

130+
MP4::Atom::Atom(File *file)
131+
: Atom(file, 0)
132+
{
133+
}
134+
125135
MP4::Atom::~Atom() = default;
126136

127137
MP4::Atom *

taglib/mp4/mp4atom.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ namespace TagLib {
8989
const ByteVector &name() const;
9090
const AtomList &children() const;
9191

92+
protected:
93+
Atom(File *file, int depth);
94+
9295
private:
9396
class AtomPrivate;
9497
TAGLIB_MSVC_SUPPRESS_WARNING_NEEDS_TO_HAVE_DLL_INTERFACE

0 commit comments

Comments
 (0)