From f9bfec180f91671d8ba72a01cab1781c1f5e9999 Mon Sep 17 00:00:00 2001
From: ejurgensen <espenjurgensen@gmail.com>
Date: Sat, 10 Aug 2019 22:41:04 +0200
Subject: [PATCH] [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
---
 src/outputs/raop.c       | 13 +++++++------
 src/outputs/rtp_common.c |  7 ++++++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/outputs/raop.c b/src/outputs/raop.c
index c4cf3b28..fdef8311 100644
--- a/src/outputs/raop.c
+++ b/src/outputs/raop.c
@@ -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
diff --git a/src/outputs/rtp_common.c b/src/outputs/rtp_common.c
index 4db706dd..a5b4cf3c 100644
--- a/src/outputs/rtp_common.c
+++ b/src/outputs/rtp_common.c
@@ -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];
 }