[http] Avoid potentially comparing TCP header bytes against CRLF

The optimisation to check for a trailing CRLF in http_rx_chunk_data()
could potentially underflow and look for the CR and LF bytes in the
I/O buffer data that immediately precedes the HTTP content (i.e. in
the TCP header).

Fix by avoiding the potential underflow.  Update the code to use a
dedicated CRLF structure, to reduce the proliferation of magic numbers
within the function.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/net/tcp/httpcore.c b/src/net/tcp/httpcore.c
index 7dbb0e3044b31364e0ef231a931f62b1d9d5923a..affb482651c0f60866661fda728fe7f479f7db38 100644 (file)
--- a/src/net/tcp/httpcore.c
+++ b/src/net/tcp/httpcore.c
@@ -1808,8 +1808,11 @@ static int http_rx_chunk_len ( struct http_transaction *http,
  */
 static int http_rx_chunk_data ( struct http_transaction *http,
                                struct io_buffer **iobuf ) {
+       struct {
+               uint8_t cr;
+               uint8_t lf;
+       } __attribute__ (( packed )) *crlf;
        struct io_buffer *payload;
-       uint8_t *crlf;
        size_t len;
        int rc;
 
@@ -1818,12 +1821,16 @@ static int http_rx_chunk_data ( struct http_transaction *http,
         * (which we would ignore anyway) and hence avoid
         * unnecessarily copying the data.
         */
-       if ( iob_len ( *iobuf ) == ( http->remaining + 2 /* CRLF */ ) ) {
+       len = iob_len ( *iobuf );
+       if ( ( len >= sizeof ( *crlf ) ) &&
+            ( ( len - sizeof ( *crlf ) ) == http->remaining ) ) {
                crlf = ( (*iobuf)->data + http->remaining );
-               if ( ( crlf[0] == '\r' ) && ( crlf[1] == '\n' ) )
-                       iob_unput ( (*iobuf), 2 /* CRLF */ );
+               if ( ( crlf->cr == '\r' ) && ( crlf->lf == '\n' ) ) {
+                       iob_unput ( *iobuf, sizeof ( *crlf ) );
+                       len -= sizeof ( *crlf );
+               }
        }
-       len = iob_len ( *iobuf );
+       assert ( len == iob_len ( *iobuf ) );
 
        /* Use whole/partial buffer as applicable */
        if ( len <= http->remaining ) {