Merge pull request #10966 from ejohnstown/upd-ocsp

Reject CR/LF and obs-fold in OCSP requests

Author: JacobBarthelmeh

src/ocsp.c

@@ -1770,6 +1770,20 @@ WOLFSSL_OCSP_REQ_CTX* wolfSSL_OCSP_sendreq_new(WOLFSSL_BIO *bio,
     return NULL;
 }
 
+/* Returns 0 if the string has no CR or LF, -1 if it does. */
+static int OCSP_REQ_CTX_no_crlf(const char* value)
+{
+    if (value != NULL) {
+        const char* c;
+        for (c = value; *c != '\0'; c++) {
+            if (*c == '\r' || *c == '\n')
+                return -1;
+        }
+    }
+
+    return 0;
+}
+
 int wolfSSL_OCSP_REQ_CTX_add1_header(WOLFSSL_OCSP_REQ_CTX *ctx,
                              const char *name, const char *value)
 {
@@ -1779,6 +1793,16 @@ int wolfSSL_OCSP_REQ_CTX_add1_header(WOLFSSL_OCSP_REQ_CTX *ctx,
         WOLFSSL_MSG("Bad parameter");
         return WOLFSSL_FAILURE;
     }
+    if (OCSP_REQ_CTX_no_crlf(name) != 0 || OCSP_REQ_CTX_no_crlf(value) != 0) {
+        WOLFSSL_MSG("CR/LF in header name or value");
+        return WOLFSSL_FAILURE;
+    }
+    /* A name starting with whitespace is an obs-fold continuation line
+     * (RFC 7230 Section 3.2.4) appending to the previous header's value. */
+    if (*name == ' ' || *name == '\t') {
+        WOLFSSL_MSG("Leading whitespace in header name");
+        return WOLFSSL_FAILURE;
+    }
     if (wolfSSL_BIO_puts(ctx->reqResp, name) <= 0) {
         WOLFSSL_MSG("wolfSSL_BIO_puts error");
         return WOLFSSL_FAILURE;
@@ -1818,6 +1842,11 @@ int wolfSSL_OCSP_REQ_CTX_http(WOLFSSL_OCSP_REQ_CTX *ctx, const char *op,
     if (path == NULL)
         path = "/";
 
+    if (OCSP_REQ_CTX_no_crlf(op) != 0 || OCSP_REQ_CTX_no_crlf(path) != 0) {
+        WOLFSSL_MSG("CR/LF in HTTP op or path");
+        return WOLFSSL_FAILURE;
+    }
+
     if (wolfSSL_BIO_printf(ctx->reqResp, http_hdr, op, path) <= 0) {
         WOLFSSL_MSG("WOLFSSL_OCSP_REQ_CTX: wolfSSL_BIO_printf error");
         return WOLFSSL_FAILURE;

src/ssl_api_crl_ocsp.c

@@ -650,6 +650,7 @@ int wolfSSL_OCSP_parse_url(const char* url, char** host, char** port,
         char** path, int* ssl)
 {
     const char* u;
+    const char* c;
     const char* upath; /* path in u */
     const char* uport; /* port in u */
     const char* hostEnd;
@@ -667,6 +668,15 @@ int wolfSSL_OCSP_parse_url(const char* url, char** host, char** port,
     *path = NULL;
     *ssl = 0;
 
+    /* CR/LF in the parsed out host or path would split a request built from
+     * them into extra header lines. */
+    for (c = url; *c != '\0'; c++) {
+        if (*c == '\r' || *c == '\n') {
+            WOLFSSL_MSG("CR/LF in URL");
+            goto err;
+        }
+    }
+
     if (*(u++) != 'h') goto err;
     if (*(u++) != 't') goto err;
     if (*(u++) != 't') goto err;

tests/api.c

@@ -22691,6 +22691,13 @@ static int test_wolfSSL_OCSP_parse_url(void)
     CK_OPU_FAIL("http/""/localhost");
     CK_OPU_FAIL("http:/localhost");
     CK_OPU_FAIL("https://localhost/path:1234");
+    /* CR/LF anywhere in the URL, paired or bare. */
+    CK_OPU_FAIL("http://localhost/\r\nX-Injected: yes");
+    CK_OPU_FAIL("http://localhost\r\n:1234/");
+    CK_OPU_FAIL("http://localhost/\nX-Injected: yes");
+    CK_OPU_FAIL("http://localhost/\rX-Injected: yes");
+    CK_OPU_FAIL("http://localhost\n:1234/");
+    CK_OPU_FAIL("http://localhost:12\r\n34/");
 
 #undef CK_OPU_OK
 #undef CK_OPU_FAIL
@@ -23035,6 +23042,28 @@ static int test_wolfSSL_OCSP_REQ_CTX(void)
     ExpectNotNull(OCSP_request_add0_id(req, cid));
     ExpectIntEQ(OCSP_request_add1_nonce(req, NULL, -1), 1);
 
+    ExpectNull(OCSP_sendreq_new(bio1, "/\r\nX-Injected: yes", NULL, -1));
+    ExpectNull(OCSP_sendreq_new(bio1, "/\nX-Injected: yes", NULL, -1));
+
+    /* Reject CR/LF and obs-fold, on a context that is thrown away so that the
+     * request assembled below stays clean. */
+    ExpectNotNull(ctx = OCSP_sendreq_new(bio1, "/", NULL, -1));
+    ExpectIntEQ(OCSP_REQ_CTX_http(ctx, "POST", "/\r\nX-Injected: yes"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_http(ctx, "POST\r\nX-Injected: yes", "/"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "X-Injected\r\nHost", "h"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "Host", "h\r\nX-Injected: yes"),
+            0);
+    ExpectIntEQ(OCSP_REQ_CTX_http(ctx, "POST", "/\nX-Injected: yes"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_http(ctx, "POST", "/\rX-Injected: yes"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "Host", "h\nX-Injected: yes"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "Host", "h\rX-Injected: yes"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, " Host", "h"), 0);
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "\tHost", "h"), 0);
+    /* A NULL value is allowed and emits just the name. */
+    ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "X-No-Value", NULL), 1);
+    OCSP_REQ_CTX_free(ctx);
+    ctx = NULL;
+
     ExpectNotNull(ctx = OCSP_sendreq_new(bio1, "/", NULL, -1));
     ExpectIntEQ(OCSP_REQ_CTX_add1_header(ctx, "Host", "127.0.0.1"), 1);
     ExpectIntEQ(OCSP_REQ_CTX_set1_req(ctx, req), 1);