From 035642748f243a0a6952b15627d16102b3254c93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Wed, 17 Jul 2019 17:03:17 +0200
Subject: [PATCH] TgaImporter: properly handle files with too short data.

---
 doc/changelog.dox                             |  2 ++
 .../TgaImporter/Test/TgaImporterTest.cpp      | 34 +++++++++++++------
 src/MagnumPlugins/TgaImporter/TgaImporter.cpp | 10 ++++--
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/doc/changelog.dox b/doc/changelog.dox
index ad7b82e10..854d7b194 100644
--- a/doc/changelog.dox
+++ b/doc/changelog.dox
@@ -430,6 +430,8 @@ See also:
     Qt instead of @ref GL::defaultFramebuffer (see
     [mosra/magnum#322](https://github.com/mosra/magnum/issues/322) and
     [mosra/magnum-bootstrap#15](https://github.com/mosra/magnum-bootstrap/pull/15))
+-   @ref Trade::TgaImporter "TgaImporter" now properly handles files with too
+    short data (see [mosra/magnum#359](https://github.com/mosra/magnum/issues/359))
 
 @subsection changelog-latest-docs Documentation
 
diff --git a/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp b/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp
index 753201ac2..d3ed2fe32 100644
--- a/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp
+++ b/src/MagnumPlugins/TgaImporter/Test/TgaImporterTest.cpp
@@ -43,7 +43,8 @@ struct TgaImporterTest: TestSuite::Tester {
     explicit TgaImporterTest();
 
     void openEmpty();
-    void openShort();
+    void openShortHeader();
+    void openShortData();
 
     void paletted();
     void compressed();
@@ -64,7 +65,8 @@ struct TgaImporterTest: TestSuite::Tester {
 
 TgaImporterTest::TgaImporterTest() {
     addTests({&TgaImporterTest::openEmpty,
-              &TgaImporterTest::openShort,
+              &TgaImporterTest::openShortHeader,
+              &TgaImporterTest::openShortData,
 
               &TgaImporterTest::paletted,
               &TgaImporterTest::compressed,
@@ -97,7 +99,7 @@ void TgaImporterTest::openEmpty() {
     CORRADE_COMPARE(out.str(), "Trade::TgaImporter::openData(): the file is empty\n");
 }
 
-void TgaImporterTest::openShort() {
+void TgaImporterTest::openShortHeader() {
     Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
     const char data[] = { 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
     CORRADE_VERIFY(importer->openData(data));
@@ -108,6 +110,24 @@ void TgaImporterTest::openShort() {
     CORRADE_COMPARE(debug.str(), "Trade::TgaImporter::image2D(): the file is too short: 17 bytes\n");
 }
 
+constexpr const char ColorBits24[] = {
+    0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0,
+    1, 2, 3, 2, 3, 4,
+    3, 4, 5, 4, 5, 6,
+    5, 6, 7, 6, 7, 8
+};
+
+void TgaImporterTest::openShortData() {
+    Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
+
+    CORRADE_VERIFY(importer->openData(Containers::arrayView(ColorBits24).except(1)));
+
+    std::ostringstream debug;
+    Error redirectError{&debug};
+    CORRADE_VERIFY(!importer->image2D(0));
+    CORRADE_COMPARE(debug.str(), "Trade::TgaImporter::image2D(): the file is too short: got 35 bytes but expected 36\n");
+}
+
 void TgaImporterTest::paletted() {
     Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
     const char data[] = { 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };
@@ -143,18 +163,12 @@ void TgaImporterTest::colorBits16() {
 
 void TgaImporterTest::colorBits24() {
     Containers::Pointer<AbstractImporter> importer = _manager.instantiate("TgaImporter");
-    const char data[] = {
-        0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 3, 0, 24, 0,
-        1, 2, 3, 2, 3, 4,
-        3, 4, 5, 4, 5, 6,
-        5, 6, 7, 6, 7, 8
-    };
     const char pixels[] = {
         3, 2, 1, 4, 3, 2,
         5, 4, 3, 6, 5, 4,
         7, 6, 5, 8, 7, 6
     };
-    CORRADE_VERIFY(importer->openData(data));
+    CORRADE_VERIFY(importer->openData(ColorBits24));
 
     Containers::Optional<Trade::ImageData2D> image = importer->image2D(0);
     CORRADE_VERIFY(image);
diff --git a/src/MagnumPlugins/TgaImporter/TgaImporter.cpp b/src/MagnumPlugins/TgaImporter/TgaImporter.cpp
index 3b6ef9513..2e5ed082b 100644
--- a/src/MagnumPlugins/TgaImporter/TgaImporter.cpp
+++ b/src/MagnumPlugins/TgaImporter/TgaImporter.cpp
@@ -119,8 +119,14 @@ Containers::Optional<ImageData2D> TgaImporter::doImage2D(UnsignedInt) {
         return Containers::NullOpt;
     }
 
-    Containers::Array<char> data{std::size_t(size.product())*header.bpp/8};
-    std::copy_n(_in + sizeof(Implementation::TgaHeader), data.size(), data.begin());
+    std::size_t outputSize = std::size_t(size.product())*header.bpp/8;
+    if(outputSize + sizeof(Implementation::TgaHeader) > _in.size()) {
+        Error{} << "Trade::TgaImporter::image2D(): the file is too short: got" << _in.size() << "bytes but expected" << outputSize + sizeof(Implementation::TgaHeader);
+        return Containers::NullOpt;
+    }
+
+    Containers::Array<char> data{outputSize};
+    std::copy_n(_in + sizeof(Implementation::TgaHeader), outputSize, data.begin());
 
     /* Adjust pixel storage if row size is not four byte aligned */
     PixelStorage storage;