Skip to content

Commit dcc4a38

Browse files
yosuke-wolfsslyosuke-wolfssl
committed
Reject CR/LF in OCSP/CRL URLs to block HTTP injection
1 parent 8fca95c commit dcc4a38

4 files changed

Lines changed: 123 additions & 6 deletions

File tree

‎src/wolfio.c‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,7 +1680,7 @@ int wolfIO_TcpAccept(SOCKET_T sockfd, SOCKADDR* peer_addr, XSOCKLENT* peer_len)
16801680
int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
16811681
word16* outPort)
16821682
{
1683-
int result = -1;
1683+
int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR);
16841684

16851685
if (url == NULL || urlSz == 0) {
16861686
if (outName)
@@ -1706,6 +1706,8 @@ int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
17061706
/* copy until ']' */
17071707
while (i < MAX_URL_ITEM_SIZE-1 && cur < urlSz && url[cur] != 0 &&
17081708
url[cur] != ']') {
1709+
if (url[cur] == '\r' || url[cur] == '\n')
1710+
return WOLFSSL_FATAL_ERROR;
17091711
if (outName)
17101712
outName[i] = url[cur];
17111713
i++; cur++;
@@ -1715,6 +1717,8 @@ int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
17151717
else {
17161718
while (i < MAX_URL_ITEM_SIZE-1 && cur < urlSz && url[cur] != 0 &&
17171719
url[cur] != ':' && url[cur] != '/') {
1720+
if (url[cur] == '\r' || url[cur] == '\n')
1721+
return WOLFSSL_FATAL_ERROR;
17181722
if (outName)
17191723
outName[i] = url[cur];
17201724
i++; cur++;
@@ -1752,6 +1756,8 @@ int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
17521756
if (cur < urlSz && url[cur] == '/') {
17531757
i = 0;
17541758
while (i < MAX_URL_ITEM_SIZE-1 && cur < urlSz && url[cur] != 0) {
1759+
if (url[cur] == '\r' || url[cur] == '\n')
1760+
return WOLFSSL_FATAL_ERROR;
17551761
if (outPath)
17561762
outPath[i] = url[cur];
17571763
i++; cur++;
@@ -2065,6 +2071,21 @@ int wolfIO_HttpBuildRequest(const char *reqType, const char *domainName,
20652071
return wolfIO_HttpBuildRequest_ex(reqType, domainName, path, pathLen, reqSz, contentType, "", buf, bufSize);
20662072
}
20672073

2074+
/* Returns 1 if the buffer contains a CR or LF byte, 0 otherwise. Used to
2075+
* reject attacker-controlled URL components that would allow HTTP header
2076+
* injection or request splitting. */
2077+
static int wolfIO_UrlHasCrlf(const char* in, int inSz)
2078+
{
2079+
int i = 0;
2080+
if (in == NULL)
2081+
return 0;
2082+
for (i = 0; i < inSz; i++) {
2083+
if (in[i] == '\r' || in[i] == '\n')
2084+
return 1;
2085+
}
2086+
return 0;
2087+
}
2088+
20682089
int wolfIO_HttpBuildRequest_ex(const char *reqType, const char *domainName,
20692090
const char *path, int pathLen, int reqSz, const char *contentType,
20702091
const char *exHdrs, byte *buf, int bufSize)
@@ -2087,6 +2108,13 @@ int wolfIO_HttpBuildRequest_ex(const char *reqType, const char *domainName,
20872108
reqSzStrLen = wolfIO_Word16ToString(reqSzStr, (word16)reqSz);
20882109
contentTypeLen = (word32)XSTRLEN(contentType);
20892110

2111+
/* reject CR/LF in the path or host to prevent HTTP header injection or
2112+
* request splitting from attacker-controlled URL components */
2113+
if (wolfIO_UrlHasCrlf(path, pathLen) ||
2114+
wolfIO_UrlHasCrlf(domainName, (int)domainNameLen)) {
2115+
return 0;
2116+
}
2117+
20902118
blankStrLen = (word32)XSTRLEN(blankStr);
20912119
http11StrLen = (word32)XSTRLEN(http11Str);
20922120
hostStrLen = (word32)XSTRLEN(hostStr);
@@ -2126,7 +2154,10 @@ int wolfIO_HttpBuildRequest_ex(const char *reqType, const char *domainName,
21262154
buf += reqTypeLen; bufSize -= (int)reqTypeLen;
21272155
XSTRNCPY((char*)buf, blankStr, (size_t)bufSize);
21282156
buf += blankStrLen; bufSize -= (int)blankStrLen;
2129-
XSTRNCPY((char*)buf, path, (size_t)bufSize);
2157+
/* path may not be NUL terminated (CRL raw-URL case passes a pointer into
2158+
* the certificate with only pathLen valid bytes), so bound the copy to
2159+
* pathLen rather than scanning for a NUL */
2160+
XMEMCPY((char*)buf, path, (size_t)pathLen);
21302161
buf += pathLen; bufSize -= (int)pathLen;
21312162
XSTRNCPY((char*)buf, http11Str, (size_t)bufSize);
21322163
buf += http11StrLen; bufSize -= (int)http11StrLen;
@@ -2251,8 +2282,11 @@ int EmbedOcspLookup(void* ctx, const char* url, int urlSz,
22512282
httpBufSz = wolfIO_HttpBuildRequestOcsp(domainName, path, ocspReqSz,
22522283
httpBuf, httpBufSz);
22532284

2254-
ret = wolfIO_TcpConnect(&sfd, domainName, port, io_timeout_sec);
2255-
if (ret != 0) {
2285+
if (httpBufSz <= 0) {
2286+
WOLFSSL_MSG("Unable to build OCSP request");
2287+
}
2288+
else if ((ret = wolfIO_TcpConnect(&sfd, domainName, port,
2289+
io_timeout_sec)) != 0) {
22562290
WOLFSSL_MSG("OCSP Responder connection failed");
22572291
}
22582292
else if (wolfIO_Send(sfd, (char*)httpBuf, httpBufSz, 0) !=
@@ -2351,8 +2385,11 @@ int EmbedCrlLookup(WOLFSSL_CRL* crl, const char* url, int urlSz)
23512385
httpBufSz = wolfIO_HttpBuildRequestCrl(url, urlSz, domainName,
23522386
httpBuf, httpBufSz);
23532387

2354-
ret = wolfIO_TcpConnect(&sfd, domainName, port, io_timeout_sec);
2355-
if (ret != 0) {
2388+
if (httpBufSz <= 0) {
2389+
WOLFSSL_MSG("Unable to build CRL request");
2390+
}
2391+
else if ((ret = wolfIO_TcpConnect(&sfd, domainName, port,
2392+
io_timeout_sec)) != 0) {
23562393
WOLFSSL_MSG("CRL connection failed");
23572394
}
23582395
else if (wolfIO_Send(sfd, (char*)httpBuf, httpBufSz, 0)

‎tests/api.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35240,6 +35240,7 @@ TEST_CASE testCases[] = {
3524035240
TEST_DECL(test_ocsp_cert_unknown_crl_fallback_nonleaf),
3524135241
TEST_DECL(test_tls13_nonblock_ocsp_low_mfl),
3524235242
TEST_DECL(test_ocsp_responder),
35243+
TEST_DECL(test_wolfIO_DecodeUrl_crlf_reject),
3524335244
TEST_TLS_DECLS,
3524435245
TEST_SESSION_DECLS,
3524535246
TEST_DECL(test_wc_DhSetNamedKey),

‎tests/api/test_ocsp.c‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,3 +1809,81 @@ int test_ocsp_responder(void)
18091809
return TEST_SKIPPED;
18101810
}
18111811
#endif /* HAVE_OCSP_RESPONDER && !NO_SHA && !NO_RSA */
1812+
1813+
#if defined(HAVE_HTTP_CLIENT)
1814+
/* A peer-supplied AIA/CRL URL must not be able to smuggle CR/LF into the
1815+
* outbound OCSP/CRL HTTP request (header injection / request splitting). */
1816+
int test_wolfIO_DecodeUrl_crlf_reject(void)
1817+
{
1818+
EXPECT_DECLS;
1819+
char domainName[80];
1820+
char path[80];
1821+
word16 port;
1822+
byte reqBuf[512];
1823+
char* tightPath = NULL;
1824+
int tightLen = 16;
1825+
const char* crlfInPath = "http://ocsp.example.com/ocsp\r\nX-Injected: 1";
1826+
const char* crlfInHost = "http://evil\r\nHost: x/ocsp";
1827+
const char* crlfInV6Host = "http://[::1\r\n]/ocsp";
1828+
const char* cleanUrl = "http://ocsp.example.com/ocsp";
1829+
const char* taintedPath = "/ocsp\r\nX-Injected: 1";
1830+
const char* crlfAfterCap =
1831+
"http://ocsp.example.com/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\r\nX: 1";
1832+
1833+
/* parser rejects CR/LF in the path */
1834+
ExpectIntLT(wolfIO_DecodeUrl(crlfInPath, (int)XSTRLEN(crlfInPath),
1835+
domainName, path, &port), 0);
1836+
1837+
/* parser rejects CR/LF in the host */
1838+
ExpectIntLT(wolfIO_DecodeUrl(crlfInHost, (int)XSTRLEN(crlfInHost),
1839+
domainName, path, &port), 0);
1840+
1841+
/* parser rejects CR/LF in an IPv6 bracket-literal host */
1842+
ExpectIntLT(wolfIO_DecodeUrl(crlfInV6Host, (int)XSTRLEN(crlfInV6Host),
1843+
domainName, path, &port), 0);
1844+
1845+
/* sink rejects a tainted path even when the parser is bypassed, which is
1846+
* the CRL raw-URL case */
1847+
ExpectIntEQ(wolfIO_HttpBuildRequest("POST", "ocsp.example.com",
1848+
taintedPath, (int)XSTRLEN(taintedPath), 0, "application/ocsp-request",
1849+
reqBuf, (int)sizeof(reqBuf)), 0);
1850+
1851+
/* sink rejects CR/LF even when it appears beyond the decoder cap */
1852+
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", "ocsp.example.com",
1853+
crlfAfterCap, (int)XSTRLEN(crlfAfterCap), 0, "", reqBuf,
1854+
(int)sizeof(reqBuf)), 0);
1855+
1856+
/* sink rejects CR/LF in the host (domainName) with a clean path */
1857+
ExpectIntEQ(wolfIO_HttpBuildRequest("POST", "evil.example.com\r\nX: 1",
1858+
"/ocsp", (int)XSTRLEN("/ocsp"), 0, "application/ocsp-request", reqBuf,
1859+
(int)sizeof(reqBuf)), 0);
1860+
1861+
/* the CRL raw-URL path is not NUL terminated, so the request builder must
1862+
* copy only pathLen bytes. Allocate exactly pathLen bytes with no
1863+
* terminator so an over-read past the buffer is caught under ASAN. */
1864+
tightPath = (char*)XMALLOC((size_t)tightLen, NULL, DYNAMIC_TYPE_TMP_BUFFER);
1865+
ExpectNotNull(tightPath);
1866+
if (tightPath != NULL) {
1867+
XMEMSET(tightPath, 'a', (size_t)tightLen);
1868+
tightPath[0] = '/';
1869+
ExpectIntGT(wolfIO_HttpBuildRequest("GET", "ocsp.example.com",
1870+
tightPath, tightLen, 0, "", reqBuf, (int)sizeof(reqBuf)), 0);
1871+
}
1872+
1873+
/* positive control: a clean URL decodes and still builds a request */
1874+
ExpectIntEQ(wolfIO_DecodeUrl(cleanUrl, (int)XSTRLEN(cleanUrl), domainName,
1875+
path, &port), 0);
1876+
ExpectIntGT(wolfIO_HttpBuildRequest("POST", domainName, path,
1877+
(int)XSTRLEN(path), 0, "application/ocsp-request", reqBuf,
1878+
(int)sizeof(reqBuf)), 0);
1879+
1880+
XFREE(tightPath, NULL, DYNAMIC_TYPE_TMP_BUFFER);
1881+
1882+
return EXPECT_RESULT();
1883+
}
1884+
#else
1885+
int test_wolfIO_DecodeUrl_crlf_reject(void)
1886+
{
1887+
return TEST_SKIPPED;
1888+
}
1889+
#endif /* HAVE_HTTP_CLIENT */

‎tests/api/test_ocsp.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ int test_ocsp_cert_unknown_crl_fallback_nonleaf(void);
3535
int test_tls13_nonblock_ocsp_low_mfl(void);
3636
int test_ocsp_responder(void);
3737
int test_ocsp_ancestor_responder_rejected(void);
38+
int test_wolfIO_DecodeUrl_crlf_reject(void);
3839
#endif /* WOLFSSL_TEST_OCSP_H */
3940

0 commit comments

Comments
 (0)