[Spice-devel] [PATCH spice-server 03/30] Terminate HTTP header

[Frediano Ziglio]

Allow to use standard string functions and prevent some
possible overflow (like the use of strchr).
---
 server/reds-stream.c |  7 ++++---
 server/websocket.c   | 39 +++++++++++++++++----------------------
 server/websocket.h   |  4 ++--
 3 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/server/reds-stream.c b/server/reds-stream.c
index 4327e57..b59586b 100644
--- a/server/reds-stream.c
+++ b/server/reds-stream.c
@@ -1202,10 +1202,11 @@ bool reds_stream_is_websocket(RedsStream *stream, unsigned char *buf, int len)
     }
 
     memcpy(rbuf, buf, len);
-    rc = stream->priv->read(stream, rbuf + len, sizeof(rbuf) - len);
+    rc = stream->priv->read(stream, rbuf + len, sizeof(rbuf) - len - 1);
     if (rc <= 0)
         return FALSE;
     len += rc;
+    rbuf[len] = 0;
 
     /* TODO:  this has a theoretical flaw around packet buffering
               that is not likely to occur in practice.  That is,
@@ -1223,10 +1224,10 @@ bool reds_stream_is_websocket(RedsStream *stream, unsigned char *buf, int len)
               so it seems wisest to live with this theoretical flaw.
     */
 
-    if (websocket_is_start(rbuf, len)) {
+    if (websocket_is_start(rbuf)) {
         char outbuf[1024];
 
-        websocket_create_reply(rbuf, len, outbuf);
+        websocket_create_reply(rbuf, outbuf);
         rc = stream->priv->write(stream, outbuf, strlen(outbuf));
         if (rc == strlen(outbuf)) {
             stream->priv->ws = spice_malloc0(sizeof(*stream->priv->ws));
diff --git a/server/websocket.c b/server/websocket.c
index 6dbf354..cfd93a0 100644
--- a/server/websocket.c
+++ b/server/websocket.c
@@ -15,6 +15,7 @@
    You should have received a copy of the GNU Lesser General Public
    License along with this library; if not, see <http://www.gnu.org/licenses/>.
 */
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdbool.h>
@@ -55,16 +56,12 @@
 /* Perform a case insenstive search for needle in haystack.
    If found, return a pointer to the byte after the end of needle.
    Otherwise, return NULL */
-static gchar *find_str(gchar *haystack, const char *needle, int n)
+static gchar *find_str(gchar *haystack, const char *needle)
 {
-    int i;
-
-    for (i = 0; i < n; i++) {
-        if ((n - i) < strlen(needle))
-            break;
+    char *s = strcasestr(haystack, needle);
 
-        if (g_ascii_strncasecmp(haystack + i, needle, strlen(needle)) == 0)
-            return haystack + i + strlen(needle);
+    if (s) {
+        return s + strlen(needle);
     }
     return NULL;
 }
@@ -74,7 +71,7 @@ static gchar *find_str(gchar *haystack, const char *needle, int n)
    required to extract the length; useful for tracking where the
    mask will be.
 */
-static guint64 extract_length(guint8 *buf, int *used)
+static guint64 extract_length(const guint8 *buf, int *used)
 {
     int i;
     guint64 outlen = (*buf++) & LENGTH_MASK;
@@ -127,7 +124,7 @@ static int frame_bytes_needed(websocket_frame_t *frame)
 *  original key sent to us
 * If non null, caller must free returned key string.
 */
-static gchar *generate_reply_key(gchar *buf, int len)
+static gchar *generate_reply_key(gchar *buf)
 {
     GChecksum *checksum = NULL;
     gchar *b64 = NULL;
@@ -137,10 +134,10 @@ static gchar *generate_reply_key(gchar *buf, int len)
     gchar *p;
     gchar *k;
 
-    key = find_str(buf, "Sec-WebSocket-Key:", len);
+    key = find_str(buf, "\nSec-WebSocket-Key:");
     if (key) {
         p = strchr(key, '\r');
-        if (p && p - buf < len) {
+        if (p) {
             k = g_strndup(key, p - key);
             k = g_strstrip(k);
             checksum = g_checksum_new(G_CHECKSUM_SHA1);
@@ -432,25 +429,23 @@ void websocket_ack_close(void *opaque, websocket_write_cb_t write_cb)
     write_cb(opaque, header, sizeof(header));
 }
 
-bool websocket_is_start(gchar *buf, int len)
+bool websocket_is_start(gchar *buf)
 {
-    if (len < 4)
-        return FALSE;
-
-    if (find_str(buf, "GET", len) == (buf + 3) &&
-            find_str(buf, "Sec-WebSocket-Protocol: binary", len) &&
-            find_str(buf, "Sec-WebSocket-Key:", len) &&
-            buf[len - 2] == '\r' && buf[len - 1] == '\n')
+    if (strncmp(buf, "GET ", 4) == 0 &&
+            // TODO strip, do not assume a single space
+            find_str(buf, "\nSec-WebSocket-Protocol: binary") &&
+            find_str(buf, "\nSec-WebSocket-Key:") &&
+            g_str_has_suffix(buf, "\r\n"))
         return TRUE;
 
     return FALSE;
 }
 
-void websocket_create_reply(gchar *buf, int len, gchar *outbuf)
+void websocket_create_reply(gchar *buf, gchar *outbuf)
 {
     gchar *key;
 
-    key = generate_reply_key(buf, len);
+    key = generate_reply_key(buf);
     sprintf(outbuf, "HTTP/1.1 101 Switching Protocols\r\n"
                     "Upgrade: websocket\r\n"
                     "Connection: Upgrade\r\n"
diff --git a/server/websocket.h b/server/websocket.h
index 3561279..02aacc1 100644
--- a/server/websocket.h
+++ b/server/websocket.h
@@ -33,8 +33,8 @@ typedef ssize_t (*websocket_read_cb_t)(void *opaque, const void *buf, size_t nby
 typedef ssize_t (*websocket_write_cb_t)(void *opaque, const void *buf, size_t nbyte);
 typedef ssize_t (*websocket_writev_cb_t)(void *opaque, struct iovec *iov, int iovcnt);
 
-bool websocket_is_start(gchar *buf, int len);
-void websocket_create_reply(gchar *buf, int len, gchar *outbuf);
+bool websocket_is_start(gchar *buf);
+void websocket_create_reply(gchar *buf, gchar *outbuf);
 int websocket_read(void *opaque, guchar *buf, int len, websocket_frame_t *frame,
          websocket_read_cb_t read_cb,
          websocket_write_cb_t write_cb);
-- 
2.7.4