Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when changing values of Simple Desk via Web API #1651

Closed
jpue opened this issue Dec 29, 2024 · 2 comments
Closed

Crash when changing values of Simple Desk via Web API #1651

jpue opened this issue Dec 29, 2024 · 2 comments

Comments

@jpue
Copy link
Contributor

jpue commented Dec 29, 2024

Describe the bug
QLC+ can be reliably crashed with incorrect channel address values via the Web API.

To Reproduce

  1. Start QLC+ with -w to enable web access.
  2. Switch to the Simple Desk page.
  3. Open https://www.qlcplus.org/Test_Web_API.html in a web browser.
  4. Scroll down the web page to Simple Desk channel set.
  5. Enter any number <= 0 or greater than the maximum number of channels as Absolute DMX address (default: 4 universes * 512 channels/universe = 2048 channels => > 2048).
  6. Click on Simple Desk channel set to send the command.
  7. QLC+ crashes (Segmentation fault).

Expected behavior
Bad values should be ignored or an error message should be displayed. In any case, the program should not freeze or crash.

I came up with this working fix, but I am not sure if this is the most elegant solution with regard to the whole QLC+ source tree:

diff --git a/ui/src/simpledesk.cpp b/ui/src/simpledesk.cpp
index adf3c298c..2ba192677 100644
--- a/ui/src/simpledesk.cpp
+++ b/ui/src/simpledesk.cpp
@@ -404,7 +404,10 @@ uchar SimpleDesk::getAbsoluteChannelValue(uint address)
 
 void SimpleDesk::setAbsoluteChannelValue(uint address, uchar value)
 {
-    m_engine->setValue(address, value);
+    if (address < (((uint) m_doc->inputOutputMap()->universesCount()) * 512))
+    {
+        m_engine->setValue(address, value);
+    }
 }
 
 void SimpleDesk::resetChannel(quint32 address)

Desktop:

  • OS: Windows 10 / Ubuntu 22.04
  • QLC+ 4.13.1 (latest official release) / QLC+ 4.13.2 GIT (on December 29, 2024)
@mcallegari
Copy link
Owner

mcallegari commented Dec 29, 2024

Why do you need to put negative numbers?

I mean, are these bug reports an exercise for you or are these related to a real workflow using the web API?

@jpue
Copy link
Contributor Author

jpue commented Dec 29, 2024

I used this api to control QLC+ from an external script which should enable a workflow similar to the one suggested by #1194.
Instead of modifying the Scene Editor code, my idea was to let the hardware control the simple desk (through the websocket api) and then dump values to a scene.
By accident, I found out that this crash might happen and thought it would be good to fix it in order to make the apis more robust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants