Validate malformed input packets before decoding

engine/src/script.cpp

@@ -787,18 +787,22 @@ QString Script::handleSetFixture(const QList<QStringList>& tokens, QList<Univers
             if (address < 512)
             {
                 quint32 universe = fxi->universe();
+                if (universe >= quint32(universes.size()))
+                    return QString("Invalid universe: %1").arg(universe);
+
+                Universe *uni = universes[universe];
                 QSharedPointer<GenericFader> fader = m_fadersMap.value(universe, QSharedPointer<GenericFader>());
                 if (fader.isNull())
                 {
-                    fader = universes[universe]->requestFader();
+                    fader = uni->requestFader();
                     fader->adjustIntensity(getAttributeValue(Intensity));
                     fader->setBlendMode(blendMode());
                     fader->setParentFunctionID(this->id());
                     fader->setName(name());
                     m_fadersMap[universe] = fader;
                 }
 
-                fader->updateChannel(doc, universes[universe], fxi->id(), ch, [value, time](FadeChannel &fc)
+                fader->updateChannel(doc, uni, fxi->id(), ch, [value, time](FadeChannel &fc)
                 {
                     fc.setTarget(value);
                     fc.setFadeTime(time);

engine/test/script/script_test.cpp

@@ -25,6 +25,7 @@
 #include "mastertimer.h"
 #include "script_test.h"
 #include "universe.h"
+#include "fixture.h"
 #include "script.h"
 #include "doc.h"
 
@@ -69,4 +70,59 @@ void Script_Test::initial()
     scr.postRun(doc.masterTimer(), ua);
 }
 
+#ifndef QMLUI
+void Script_Test::setFixtureRejectsMissingUniverse()
+{
+    Doc doc(this);
+
+    Fixture *fxi = new Fixture(&doc);
+    fxi->setUniverse(1);
+    fxi->setAddress(0);
+    fxi->setChannels(1);
+    // 99 is the fixture ID referenced by the setfixture token below.
+    QVERIFY(doc.addFixture(fxi, 99));
+
+    GrandMaster *gm = new GrandMaster();
+    QList<Universe*> ua;
+    ua.append(new Universe(0, gm));
+
+    Script scr(&doc);
+    QList<QStringList> tokens;
+    tokens << (QStringList() << Script::setFixtureCmd << "99");
+    tokens << (QStringList() << "value" << "255");
+    tokens << (QStringList() << "channel" << "0");
+
+    QCOMPARE(scr.handleSetFixture(tokens, ua), QString("Invalid universe: 1"));
+
+    qDeleteAll(ua);
+    delete gm;
+}
+
+void Script_Test::setFixtureAcceptsPresentUniverse()
+{
+    Doc doc(this);
+
+    Fixture *fxi = new Fixture(&doc);
+    fxi->setUniverse(0);
+    fxi->setAddress(0);
+    fxi->setChannels(1);
+    QVERIFY(doc.addFixture(fxi, 99));
+
+    GrandMaster *gm = new GrandMaster();
+    QList<Universe*> ua;
+    ua.append(new Universe(0, gm));
+
+    Script scr(&doc);
+    QList<QStringList> tokens;
+    tokens << (QStringList() << Script::setFixtureCmd << "99");
+    tokens << (QStringList() << "value" << "255");
+    tokens << (QStringList() << "channel" << "0");
+
+    QCOMPARE(scr.handleSetFixture(tokens, ua), QString());
+
+    qDeleteAll(ua);
+    delete gm;
+}
+#endif
+
 QTEST_MAIN(Script_Test)

engine/test/script/script_test.h

@@ -29,6 +29,10 @@ class Script_Test final : public QObject
 private slots:
     void initTestCase();
     void initial();
+#ifndef QMLUI
+    void setFixtureRejectsMissingUniverse();
+    void setFixtureAcceptsPresentUniverse();
+#endif
 };
 
 #endif

plugins/E1.31/CMakeLists.txt

@@ -52,3 +52,7 @@ if (UNIX AND NOT APPLE)
    install(FILES "${CMAKE_CURRENT_SOURCE_DIR}/org.qlcplus.QLCPlus.e131.metainfo.xml"
            DESTINATION ${METAINFODIR})
 endif()
+
+if(NOT ANDROID AND NOT IOS)
+    add_subdirectory(test)
+endif()

plugins/E1.31/e131packetizer.cpp

@@ -201,8 +201,8 @@ void E131Packetizer::setupE131Dmx(QByteArray& data, const int &universe, const i
 
 bool E131Packetizer::checkPacket(QByteArray &data)
 {
-    /* An E1.31 packet must be at least 125 bytes long */
-    if (data.length() < 125)
+    /* An E1.31 DMX packet must include the DMX512-A start code. */
+    if (data.length() < 126)
         return false;
 
     // check ACN packet identifier
@@ -225,7 +225,8 @@ bool E131Packetizer::checkPacket(QByteArray &data)
 
 bool E131Packetizer::fillDMXdata(QByteArray& data, QByteArray &dmx, quint32 &universe)
 {
-    if (data.isNull())
+    // Keep this self-contained even though callers normally run checkPacket().
+    if (data.isNull() || data.length() < 126)
         return false;
 
     /* Check valid DMX start code */
@@ -235,6 +236,9 @@ bool E131Packetizer::fillDMXdata(QByteArray& data, QByteArray &dmx, quint32 &uni
     universe = (uchar(data[113]) << 8) + uchar(data[114]);
     int length = (uchar(data[123]) << 8) + uchar(data[124]);
 
+    if (length < 2 || length > 513 || length > data.length() - 125)
+        return false;
+
     qDebug() << "[E1.31 fillDMXdata] universe:" << universe << ", length:" << length - 1;
 
     dmx.clear();

plugins/E1.31/test/CMakeLists.txt

@@ -0,0 +1,14 @@
+add_executable(e131packetizer_test WIN32 MACOSX_BUNDLE
+    ../e131packetizer.cpp ../e131packetizer.h
+    e131packetizer_test.cpp e131packetizer_test.h
+)
+
+target_include_directories(e131packetizer_test PRIVATE
+    ..
+)
+
+target_link_libraries(e131packetizer_test PRIVATE
+    Qt${QT_MAJOR_VERSION}::Core
+    Qt${QT_MAJOR_VERSION}::Network
+    Qt${QT_MAJOR_VERSION}::Test
+)

plugins/E1.31/test/e131packetizer_test.cpp

@@ -0,0 +1,107 @@
+/*
+  Q Light Controller Plus
+  e131packetizer_test.cpp
+
+  Copyright (c) Q Light Controller Plus
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0.txt
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+#include <QTest>
+
+#include "e131packetizer_test.h"
+#include "e131packetizer.h"
+
+void E131Packetizer_Test::checkPacketRejectsTruncatedPacket()
+{
+    E131Packetizer ep("00:11:22:33:44:55");
+    QByteArray data;
+
+    ep.setupE131Dmx(data, 0, E131_PRIORITY_DEFAULT, QByteArray());
+    QVERIFY(ep.checkPacket(data) == true);
+
+    data.truncate(125);
+    QVERIFY(ep.checkPacket(data) == false);
+}
+
+void E131Packetizer_Test::fillDMXdataRejectsZeroPropertyCount()
+{
+    E131Packetizer ep("00:11:22:33:44:55");
+    QByteArray data;
+    QByteArray dmx;
+    quint32 universe = 0;
+
+    ep.setupE131Dmx(data, 0, E131_PRIORITY_DEFAULT, QByteArray());
+    data[123] = char(0x00);
+    data[124] = char(0x00);
+
+    QVERIFY(ep.fillDMXdata(data, dmx, universe) == false);
+}
+
+void E131Packetizer_Test::fillDMXdataRejectsStartCodeOnly()
+{
+    E131Packetizer ep("00:11:22:33:44:55");
+    QByteArray data;
+    QByteArray dmx;
+    quint32 universe = 0;
+
+    ep.setupE131Dmx(data, 0, E131_PRIORITY_DEFAULT, QByteArray());
+
+    QVERIFY(ep.checkPacket(data) == true);
+    QVERIFY(ep.fillDMXdata(data, dmx, universe) == false);
+}
+
+void E131Packetizer_Test::fillDMXdataRejectsOversizedPropertyCount()
+{
+    E131Packetizer ep("00:11:22:33:44:55");
+    QByteArray data;
+    QByteArray dmx;
+    quint32 universe = 0;
+
+    ep.setupE131Dmx(data, 0, E131_PRIORITY_DEFAULT, QByteArray(512, 42));
+    data[123] = char(0x02);
+    data[124] = char(0x02);
+
+    QVERIFY(ep.fillDMXdata(data, dmx, universe) == false);
+}
+
+void E131Packetizer_Test::fillDMXdataRejectsTruncatedPayload()
+{
+    E131Packetizer ep("00:11:22:33:44:55");
+    QByteArray data;
+    QByteArray dmx;
+    quint32 universe = 0;
+
+    ep.setupE131Dmx(data, 0, E131_PRIORITY_DEFAULT, QByteArray(2, 42));
+    data.chop(1);
+
+    QVERIFY(ep.checkPacket(data) == true);
+    QVERIFY(ep.fillDMXdata(data, dmx, universe) == false);
+}
+
+void E131Packetizer_Test::fillDMXdataAcceptsValidPayload()
+{
+    E131Packetizer ep("00:11:22:33:44:55");
+    QByteArray data;
+    QByteArray dmx;
+    quint32 universe = 0;
+
+    ep.setupE131Dmx(data, 1, E131_PRIORITY_DEFAULT, QByteArray(2, 42));
+
+    QVERIFY(ep.checkPacket(data) == true);
+    QVERIFY(ep.fillDMXdata(data, dmx, universe) == true);
+    QCOMPARE(universe, quint32(1));
+    QCOMPARE(dmx, QByteArray(2, 42));
+}
+
+QTEST_MAIN(E131Packetizer_Test)

plugins/E1.31/test/e131packetizer_test.h

@@ -0,0 +1,38 @@
+/*
+  Q Light Controller Plus
+  e131packetizer_test.h
+
+  Copyright (c) Q Light Controller Plus
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0.txt
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+#ifndef E131PACKETIZER_TEST_H
+#define E131PACKETIZER_TEST_H
+
+#include <QObject>
+
+class E131Packetizer_Test final : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void checkPacketRejectsTruncatedPacket();
+    void fillDMXdataRejectsZeroPropertyCount();
+    void fillDMXdataRejectsStartCodeOnly();
+    void fillDMXdataRejectsOversizedPropertyCount();
+    void fillDMXdataRejectsTruncatedPayload();
+    void fillDMXdataAcceptsValidPayload();
+};
+
+#endif

plugins/artnet/src/artnetpacketizer.cpp

@@ -229,7 +229,8 @@ bool ArtNetPacketizer::checkPacketAndCode(QByteArray const& data, quint16 &code)
 
 bool ArtNetPacketizer::fillArtPollReplyInfo(QByteArray const& data, ArtNetNodeInfo& info)
 {
-    if (data.isNull())
+    // The fields read below extend up to style at byte offset 186.
+    if (data.isNull() || data.length() < 187)
         return false;
 
     QByteArray shortName = data.mid(26, 18);
@@ -256,7 +257,7 @@ bool ArtNetPacketizer::fillArtPollReplyInfo(QByteArray const& data, ArtNetNodeIn
 
 bool ArtNetPacketizer::fillDMXdata(QByteArray const& data, QByteArray &dmx, quint32 &universe)
 {
-    if (data.isNull())
+    if (data.isNull() || data.length() < 18)
         return false;
     dmx.clear();
     //char sequence = data.at(12);
@@ -268,6 +269,12 @@ bool ArtNetPacketizer::fillDMXdata(QByteArray const& data, QByteArray &dmx, quin
     unsigned int lsb = (data.at(17)&0xff);
     int length = (msb << 8) | lsb;
 
+    if (length < 2 || length > 512 || length > data.length() - 18)
+    {
+        qWarning() << Q_FUNC_INFO << "Invalid ArtDMX payload length" << length;
+        return false;
+    }
+
     //qDebug() << "length: " << length;
     dmx.append(data.mid(18, length));
     return true;
@@ -293,6 +300,9 @@ bool ArtNetPacketizer::processTODdata(const QByteArray &data, quint32 &universe,
 
     qDebug() << "UID count:" << uidCount;
 
+    if (data.length() < 28 + (uidCount * 6))
+        return false;
+
     for (int i = 0; i < uidCount; i++)
     {
         quint16 ESTAId;
@@ -320,4 +330,3 @@ bool ArtNetPacketizer::processRDMdata(const QByteArray &data, quint32 &universe,
     RDMProtocol rdm;
     return rdm.parsePacket(data.mid(24), values);
 }
-