Check for bad LAS header values where possible. (#3237)

Co-authored-by: Howard Butler <howard@hobu.co>

io/LasHeader.cpp

@@ -85,6 +85,13 @@ LasHeader::LasHeader() : m_fileSig(FILE_SIGNATURE), m_sourceId(0),
 }
 
 
+void LasHeader::initialize(LogPtr log, uintmax_t fileSize)
+{
+    m_log = log;
+    m_fileSize = fileSize;
+}
+
+
 void LasHeader::setSummary(const LasSummaryData& summary)
 {
     m_pointCount = summary.getTotalNumPoints();
@@ -381,12 +388,22 @@ ILeStream& operator>>(ILeStream& in, LasHeader& h)
             in >> h.m_pointCountByReturn[i];
     }
 
+    if (!h.compressed() && h.m_pointOffset > h.m_fileSize)
+        throw LasHeader::error("Invalid point offset - exceeds file size.");
+    if (!h.compressed() && h.m_pointOffset + h.m_pointCount * h.m_pointLen > h.m_fileSize)
+        throw LasHeader::error("Invalid point count. Number of points exceeds file size.");
+    if (h.m_vlrOffset > h.m_fileSize)
+        throw LasHeader::error("Invalid VLR offset - exceeds file size.");
+    if (h.m_eVlrOffset > h.m_fileSize)
+        throw LasHeader::error("Invalid extended VLR offset - exceeds file size.");
+
     // Read regular VLRs.
     in.seek(h.m_vlrOffset);
     for (size_t i = 0; i < h.m_vlrCount; ++i)
     {
         LasVLR r;
-        in >> r;
+        if (!r.read(in, h.m_pointOffset))
+            throw LasHeader::error("Invalid VLR - exceeds specified file range.");
         h.m_vlrs.push_back(std::move(r));
     }
 
@@ -397,7 +414,8 @@ ILeStream& operator>>(ILeStream& in, LasHeader& h)
         for (size_t i = 0; i < h.m_eVlrCount; ++i)
         {
             ExtLasVLR r;
-            in >> r;
+            if (!r.read(in, h.m_fileSize))
+                throw LasHeader::error("Invalid Extended VLR size - exceeds file size.");
             h.m_vlrs.push_back(std::move(r));
         }
     }

io/LasHeader.hpp

@@ -384,8 +384,7 @@ class PDAL_DLL LasHeader
     const LasVLR *findVlr(const std::string& userId, uint16_t recordId) const;
     void removeVLR(const std::string& userId, uint16_t recordId);
     void removeVLR(const std::string& userId);
-    void setLog(LogPtr log)
-        { m_log = log; }
+    void initialize(LogPtr log, uintmax_t fileSize);
     const VlrList& vlrs() const
         { return m_vlrs; }
 
@@ -394,6 +393,7 @@ class PDAL_DLL LasHeader
     friend std::ostream& operator<<(std::ostream& ostr, const LasHeader& h);
 
 private:
+    uintmax_t m_fileSize;
     std::string m_fileSig;
     uint16_t m_sourceId;
     uint16_t m_globalEncoding;

io/LasReader.cpp

@@ -44,6 +44,7 @@
 #include <pdal/PointView.hpp>
 #include <pdal/QuickInfo.hpp>
 #include <pdal/util/Extractor.hpp>
+#include <pdal/util/FileUtils.hpp>
 #include <pdal/util/IStream.hpp>
 #include <pdal/util/ProgramArgs.hpp>
 
@@ -157,6 +158,20 @@ void LasReader::handleCompressionOption()
     m_compression = compression;
 }
 
+void LasReader::createStream()
+{
+    if (m_streamIf)
+        std::cerr << "Attempt to create stream twice!\n";
+    m_streamIf.reset(new LasStreamIf(m_filename));
+    if (!m_streamIf->m_istream)
+    {
+        std::ostringstream oss;
+        oss << "Unable to open stream for '"
+            << m_filename <<"' with error '" << strerror(errno) <<"'";
+        throw pdal_error(oss.str());
+    }
+}
+
 
 void LasReader::initializeLocal(PointTableRef table, MetadataNode& m)
 {
@@ -178,8 +193,7 @@ void LasReader::initializeLocal(PointTableRef table, MetadataNode& m)
         throwError(err.what());
     }
 
-    m_header.setLog(log());
-
+    m_header.initialize(log(), FileUtils::fileSize(m_filename));
     createStream();
     std::istream *stream(m_streamIf->m_istream);
 

io/LasReader.hpp

@@ -96,19 +96,7 @@ class PDAL_DLL LasReader : public Reader, public Streamable
         { return m_header.pointCount(); }
 
 protected:
-    virtual void createStream()
-    {
-        if (m_streamIf)
-            std::cerr << "Attempt to create stream twice!\n";
-        m_streamIf.reset(new LasStreamIf(m_filename));
-        if (!m_streamIf->m_istream)
-        {
-            std::ostringstream oss;
-            oss << "Unable to open stream for '"
-                << m_filename <<"' with error '" << strerror(errno) <<"'";
-            throw pdal_error(oss.str());
-        }
-    }
+    virtual void createStream();
 
     std::unique_ptr<LasStreamIf> m_streamIf;
 

io/LasVLR.cpp

@@ -45,21 +45,22 @@ namespace pdal
 
 const uint16_t LasVLR::MAX_DATA_SIZE = 65535;
 
-ILeStream& operator>>(ILeStream& in, LasVLR& v)
+bool LasVLR::read(ILeStream& in, size_t limit)
 {
     uint16_t reserved;
     uint16_t dataLen;
 
     in >> reserved;
-    in.get(v.m_userId, 16);
-    in >> v.m_recordId >> dataLen;
-    in.get(v.m_description, 32);
-    v.m_data.resize(dataLen);
-    if (v.m_data.size() > 0) {
-        in.get(v.m_data);
-    }
-
-    return in;
+    in.get(m_userId, 16);
+    in >> m_recordId >> dataLen;
+    if ((size_t)in.position() + dataLen > limit)
+        return false;
+    in.get(m_description, 32);
+    m_data.resize(dataLen);
+    if (m_data.size() > 0)
+        in.get(m_data);
+
+    return true;
 }
 
 
@@ -208,20 +209,20 @@ OLeStream& operator<<(OLeStream& out, const LasVLR& v)
 }
 
 
-ILeStream& operator>>(ILeStream& in, ExtLasVLR& v)
+bool ExtLasVLR::read(ILeStream& in, uintmax_t limit)
 {
     uint64_t dataLen;
 
-    in >> v.m_recordSig;
-    in.get(v.m_userId, 16);
-    in >> v.m_recordId >> dataLen;
-    in.get(v.m_description, 32);
-    v.m_data.resize(dataLen);
-    if (v.m_data.size() > 0) {
-        in.get(v.m_data);
-    }
-
-    return in;
+    in >> m_recordSig;
+    in.get(m_userId, 16);
+    in >> m_recordId >> dataLen;
+    if (uintmax_t(in.position()) + dataLen > limit)
+        return false;
+    in.get(m_description, 32);
+    m_data.resize(dataLen);
+    if (m_data.size() > 0)
+        in.get(m_data);
+    return true;
 }
 
 

io/LasVLR.hpp

@@ -99,8 +99,8 @@ class PDAL_DLL LasVLR
     void setDataLen(uint64_t size)
         { m_data.resize((size_t)size); }
     void write(OLeStream& out, uint16_t recordSig);
+    bool read(ILeStream& in, size_t limit);
 
-    friend ILeStream& operator>>(ILeStream& in, LasVLR& v);
     friend OLeStream& operator<<(OLeStream& out, const LasVLR& v);
     friend std::istream& operator>>(std::istream& in, LasVLR& v);
     friend std::ostream& operator<<(std::ostream& out, const LasVLR& v);
@@ -123,9 +123,9 @@ class ExtLasVLR : public LasVLR
     ExtLasVLR()
     {}
 
-    friend ILeStream& operator>>(ILeStream& in, ExtLasVLR& v);
-    friend OLeStream& operator<<(OLeStream& out,
-        const ExtLasVLR& v);
+    bool read(ILeStream& in, uintmax_t limit);
+
+    friend OLeStream& operator<<(OLeStream& out, const ExtLasVLR& v);
     friend std::istream& operator>>(std::istream& in, ExtLasVLR& v);
     friend std::ostream& operator<<(std::ostream& out, const ExtLasVLR& v);
 };

test/unit/io/LasReaderTest.cpp

@@ -477,7 +477,7 @@ TEST(LasReaderTest, stream)
 
 // The header of 1.2-with-color-clipped says that it has 1065 points,
 // but it really only has 1064.
-TEST(LasReaderTest, LasHeaderIncorrentPointcount)
+TEST(LasReaderTest, LasHeaderIncorrectPointcount)
 {
     PointTable table;
 
@@ -486,11 +486,7 @@ TEST(LasReaderTest, LasHeaderIncorrentPointcount)
     LasReader reader;
     reader.setOptions(readOps);
 
-    reader.prepare(table);
-    PointViewSet viewSet = reader.execute(table);
-    PointViewPtr view = *viewSet.begin();
-
-    EXPECT_EQ(1064u, view->size());
+    EXPECT_THROW(reader.prepare(table), pdal_error);
 }
 
 TEST(LasReaderTest, EmptyGeotiffVlr)