Skip to content

DDP input: reject packets with unsupported data type or non-display destination #5390

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.

Sign up for GitHub

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

Open
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
+26 −25
Open

DDP input: reject packets with unsupported data type or non-display destination #5390

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
635a9e0
Initial plan
Copilot Feb 21, 2026
f884deb
Fix: reject invalid DDP packets with wrong destination or unsupported…
Copilot Feb 21, 2026
b1757c3
Move remaining DDP defines from udp.cpp to ESPAsyncE131.h
Copilot Feb 21, 2026
d2df7a4
Remove accidentally committed _codeql_detected_source_root artifact a…
Copilot Feb 21, 2026
079dbec
cleanup AI slop
DedeHai Feb 21, 2026
e02c126
missed a spot
DedeHai Feb 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions _codeql_detected_source_root
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.
6 changes: 6 additions & 0 deletions wled00/e131.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ static void handleDDPPacket(e131_packet_t* p) {
static bool ddpSeenPush = false; // have we seen a push yet?
int lastPushSeq = e131LastSequenceNumber[0];

// reject unsupported data types (only RGB and RGBW are supported)
if (p->dataType != DDP_TYPE_RGB24 && p->dataType != DDP_TYPE_RGBW32) return;
Copy link
Member

@softhack007 softhack007 Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DedeHai this is the core change, right?
Maybe its worth to make a backport to 0.15.4 with just this sanity check

Copy link
Collaborator

@DedeHai DedeHai Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that plus the next line, the rest was just moving the defines (and renaming)

Copy link
Member

@softhack007 softhack007 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read the DDP specs correctly, datatype can also be 0 = not used / undefined.
Similar for destination ID 255= all devices. I think we should allow these special values, too.

http://www.3waylabs.com/ddp/


// reject status and config packets (not implemented)
if (p->destination == DDP_ID_STATUS || p->destination == DDP_ID_CONFIG) return;

//reject late packets belonging to previous frame (assuming 4 packets max. before push)
Copy link
Member

@softhack007 softhack007 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DedeHai maybe also check if this assumption "max 4 packets per push" still makes sense.

Not sure how many LEDs fit into one packet - but considering maybe 4 packets * 512 LEDs as the absolute max content per frame doesn't sound right to me - we can support more than 2048 LEDs for example with HUB75.

Copy link
Collaborator

@DedeHai DedeHai Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs investigation. I have pushed 64x64 over websocket no problem - not sure how big one WS packet is but code comments suggest its about 500 LEDs worth.

Copy link
Member

@softhack007 softhack007 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created an issue ticket so we can come back to this question later.

if (e131SkipOutOfSequence && lastPushSeq) {
int sn = p->sequenceNum & 0xF;
Expand Down
17 changes: 17 additions & 0 deletions wled00/src/dependencies/e131/ESPAsyncE131.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,26 @@ typedef struct ip_addr ip4_addr_t;
#define DDP_PUSH_FLAG 0x01
#define DDP_TIMECODE_FLAG 0x10

#define DDP_HEADER_LEN 10
#define DDP_SYNCPACKET_LEN 10

#define DDP_FLAGS1_VER 0xc0 // version mask
#define DDP_FLAGS1_VER1 0x40 // version=1
#define DDP_FLAGS1_PUSH 0x01
#define DDP_FLAGS1_QUERY 0x02
#define DDP_FLAGS1_REPLY 0x04
#define DDP_FLAGS1_STORAGE 0x08
#define DDP_FLAGS1_TIME 0x10

#define DDP_CHANNELS_PER_PACKET 1440 // 480 leds

#define DDP_TYPE_RGB24 0x0B // 00 001 011 (RGB , 8 bits per channel, 3 channels)
#define DDP_TYPE_RGBW32 0x1B // 00 011 011 (RGBW, 8 bits per channel, 4 channels)

#define DDP_ID_DISPLAY 1
#define DDP_ID_CONFIG 250
#define DDP_ID_STATUS 251

#define ARTNET_OPCODE_OPDMX 0x5000
#define ARTNET_OPCODE_OPPOLL 0x2000
#define ARTNET_OPCODE_OPPOLLREPLY 0x2100
Expand Down
18 changes: 0 additions & 18 deletions wled00/udp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,24 +751,6 @@ void sendSysInfoUDP()
* Art-Net, DDP, E131 output - work in progress
\*********************************************************************************************/

#define DDP_HEADER_LEN 10
#define DDP_SYNCPACKET_LEN 10

#define DDP_FLAGS1_VER 0xc0 // version mask
#define DDP_FLAGS1_VER1 0x40 // version=1
#define DDP_FLAGS1_PUSH 0x01
#define DDP_FLAGS1_QUERY 0x02
#define DDP_FLAGS1_REPLY 0x04
#define DDP_FLAGS1_STORAGE 0x08
#define DDP_FLAGS1_TIME 0x10

#define DDP_ID_DISPLAY 1
#define DDP_ID_CONFIG 250
#define DDP_ID_STATUS 251

// 1440 channels per packet
#define DDP_CHANNELS_PER_PACKET 1440 // 480 leds

//
// Send real time UDP updates to the specified client
//
Expand Down