[BUG] srt-live-transmit Fails to Recover from Malformed NAK Packet #3088
Labels
Milestone
Comments
FelixSodermanNeti
added
the
Type: Bug
|
I was able to fix the issue by adding the following two lines to the updateBrokenConnection();
completeBrokenConnectionDependencies(SRT_ECONNFAIL); // LOCKS!Original Code:Before the modification, the code looked like this: if (!secure)
{
LOGC(inlog.Warn,
log << CONID() << "out-of-band LOSSREPORT received; BUG or ATTACK - last sent %" << m_iSndCurrSeqNo
<< " vs loss %" << wrong_loss);
// this should not happen: attack or bug
m_bBroken = true;
m_iBrokenCounter = 0;
return;
}Updated Code:After applying the fix, the updated code is as follows: if (!secure)
{
LOGC(inlog.Warn,
log << CONID() << "out-of-band LOSSREPORT received; BUG or ATTACK - last sent %" << m_iSndCurrSeqNo
<< " vs loss %" << wrong_loss);
// this should not happen: attack or bug
m_bBroken = true;
m_iBrokenCounter = 0;
updateBrokenConnection();
completeBrokenConnectionDependencies(SRT_ECONNFAIL); // LOCKS!
return;
}Does this seem like a reasonable fix, or could this change introduce any unintended side effects? |
|
Furthermore, I found similar code in the // Check the validation of the ack
if (CSeqNo::seqcmp(ackdata_seqno, CSeqNo::incseq(m_iSndCurrSeqNo)) > 0)
{
// this should not happen: attack or bug
LOGC(gglog.Error,
log << CONID() << "ATTACK/IPE: incoming ack seq " << ackdata_seqno << " exceeds current "
<< m_iSndCurrSeqNo << " by " << (CSeqNo::seqoff(m_iSndCurrSeqNo, ackdata_seqno) - 1) << "!");
m_bBroken = true;
m_iBrokenCounter = 0;
return;
}I haven’t attempted to reproduce this issue, but I suspect it might be vulnerable to a similar type of malformed packet. |
|
These two calls are prone to introduce deadlocks, hence one needs to be extremely careful with this. Would you be able to compile it using thread sanitizer and try to reproduce the problem on that fixed version and see what problems it reports? Note that some of the reports will come, but don't worry, many are worked around and or are false-positives. NOTE: |
|
Here are the results when running with Listener: Caller: |
|
Many thanks! I see no dangers introduced by this fix; we may need to test it also more intensively, but so far it looks good. @maxsharabayko please take a look. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Issue: srt-live-transmit Fails to Recover from Malformed NAK Packet
Describe the Bug
The
srt-live-transmitapplication fails to recover when it receives a malformed NAK (Negative Acknowledgment) packet with the "Up to sequence number" field in the Loss List set to-1(0xFFFFFFFF). Once this state is triggered, the listener enters an error state where all received bytes are dropped. Additionally, the caller cannot reconnect, even after a restart, unless the listener is manually restarted.Packet Structure:
Below is the structure of the relevant SRT header and Loss List (NAK packet):
Example Packets:
Steps to Reproduce
Use a two-way UDP proxy to drop three consecutive packets and modify the NAK packet on its return path.
UDP proxy script: Link to Proxy Script.
Run the following setup:
Listener (Source):
./srt-live-transmit udp://127.0.0.1:1111 "srt://127.0.0.1:3333?adapter=127.0.0.1&latency=120&maxbw=-1&mode=listener"Caller (Target):
./srt-live-transmit "srt://127.0.0.1:2222?latency=120&conntimeo=31393&mode=caller" udp://127.0.0.1:4444UDP Proxy:
UDP Stream Input:
(Optional) UDP Stream Monitor:
Observe logs on the listener when the malformed NAK packet is received.
Listener Logs Example:
The listener reports repeated byte losses and fails to recover:
Expected Behavior
The
srt-live-transmitlistener should handle malformed NAK packets gracefully, recover from the error state, and allow new callers to connect without requiring a restart.System Information
The text was updated successfully, but these errors were encountered: