MyFavMirrors
/
owntone-server
mirror of https://github.com/owntone/owntone-server.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[raop] Fix possible infinite loop + wrong packet resend (fixes issue #775) 

Fixes bugs which were due to incorrect handling of unsigned integer wrap-around:

1. Calling packet_resend() with seqnum + len greater than UINT16_MAX => infinite loop
2. Calling rtp_packet_get() with session->seqnum - seqnum greater than pktbuf_next => wrong packet
This commit is contained in:
ejurgensen 2019-08-10 22:41:04 +02:00
parent 6578f28621
commit f9bfec180f
2 changed files with 13 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/outputs/raop.c
View File

@ -2838,19 +2838,21 @@ control_packet_send(struct raop_session *rs, struct rtp_packet *pkt)
}
static void
packets_resend(struct raop_session *rs, uint16_t seqnum, uint16_t len)
packets_resend(struct raop_session *rs, uint16_t seqnum, int len)
{
struct rtp_session *rtp_session;
struct rtp_packet *pkt;
uint16_t s;
int i;
bool pkt_missing = false;
rtp_session = rs->master_session->rtp_session;
DPRINTF(E_DBG, L_RAOP, "Got retransmit request from '%s': seqnum %" PRIu16 " (len %" PRIu16 "), last RTP session seqnum %" PRIu16 " (len %zu)\n",
DPRINTF(E_DBG, L_RAOP, "Got retransmit request from '%s': seqnum %" PRIu16 " (len %d), last RTP session seqnum %" PRIu16 " (len %zu)\n",
rs->devname, seqnum, len, rtp_session->seqnum - 1, rtp_session->pktbuf_len);
for (s = seqnum; s < seqnum + len; s++)
// Note that seqnum may wrap around, so we don't use it for counting
for (i = 0, s = seqnum; i < len; i++, s++)
{
pkt = rtp_packet_get(rtp_session, s);
if (pkt)
@ -2860,9 +2862,8 @@ packets_resend(struct raop_session *rs, uint16_t seqnum, uint16_t len)
}
if (pkt_missing)
DPRINTF(E_WARN, L_RAOP, "Device '%s' asking for seqnum %" PRIu16 " (len %" PRIu16 "), but not in buffer\n", rs->devname, seqnum, len);
else
DPRINTF(E_DBG, L_RAOP, "Retransmit done\n");
DPRINTF(E_WARN, L_RAOP, "Device '%s' retransmit request for seqnum %" PRIu16 " (len %d) is outside buffer range (last seqnum %" PRIu16 ", len %zu)\n",
rs->devname, seqnum, len, rtp_session->seqnum - 1, rtp_session->pktbuf_len);
}
static int

7
src/outputs/rtp_common.c
View File

@ -201,6 +201,7 @@ rtp_packet_get(struct rtp_session *session, uint16_t seqnum)
{
uint16_t first;
uint16_t last;
uint16_t delta;
size_t idx;
if (!session->seqnum || !session->pktbuf_len)
@ -221,7 +222,11 @@ rtp_packet_get(struct rtp_session *session, uint16_t seqnum)
return NULL;
}
idx = (session->pktbuf_next - (session->seqnum - seqnum)) % session->pktbuf_size;
// Distance from current seqnum (which is at pktbuf_next) to the requested seqnum
delta = session->seqnum - seqnum;
// Adding pktbuf_size so we don't have to deal with "negative" pktbuf_next - delta
idx = (session->pktbuf_next + session->pktbuf_size - delta) % session->pktbuf_size;
return &session->pktbuf[idx];
}