mirror of
https://github.com/wolfSSL/wolfssl.git
synced 2026-07-09 09:40:55 +02:00
Reject CR/LF in OCSP/CRL URLs to block HTTP injection
This commit is contained in:
+49
-6
@@ -1680,7 +1680,7 @@ int wolfIO_TcpAccept(SOCKET_T sockfd, SOCKADDR* peer_addr, XSOCKLENT* peer_len)
|
||||
int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
|
||||
word16* outPort)
|
||||
{
|
||||
int result = -1;
|
||||
int result = WC_NO_ERR_TRACE(WOLFSSL_FATAL_ERROR);
|
||||
|
||||
if (url == NULL || urlSz == 0) {
|
||||
if (outName)
|
||||
@@ -1706,6 +1706,8 @@ int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
|
||||
/* copy until ']' */
|
||||
while (i < MAX_URL_ITEM_SIZE-1 && cur < urlSz && url[cur] != 0 &&
|
||||
url[cur] != ']') {
|
||||
if (url[cur] == '\r' || url[cur] == '\n')
|
||||
return WOLFSSL_FATAL_ERROR;
|
||||
if (outName)
|
||||
outName[i] = url[cur];
|
||||
i++; cur++;
|
||||
@@ -1715,6 +1717,8 @@ int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
|
||||
else {
|
||||
while (i < MAX_URL_ITEM_SIZE-1 && cur < urlSz && url[cur] != 0 &&
|
||||
url[cur] != ':' && url[cur] != '/') {
|
||||
if (url[cur] == '\r' || url[cur] == '\n')
|
||||
return WOLFSSL_FATAL_ERROR;
|
||||
if (outName)
|
||||
outName[i] = url[cur];
|
||||
i++; cur++;
|
||||
@@ -1752,6 +1756,8 @@ int wolfIO_DecodeUrl(const char* url, int urlSz, char* outName, char* outPath,
|
||||
if (cur < urlSz && url[cur] == '/') {
|
||||
i = 0;
|
||||
while (i < MAX_URL_ITEM_SIZE-1 && cur < urlSz && url[cur] != 0) {
|
||||
if (url[cur] == '\r' || url[cur] == '\n')
|
||||
return WOLFSSL_FATAL_ERROR;
|
||||
if (outPath)
|
||||
outPath[i] = url[cur];
|
||||
i++; cur++;
|
||||
@@ -2065,6 +2071,21 @@ int wolfIO_HttpBuildRequest(const char *reqType, const char *domainName,
|
||||
return wolfIO_HttpBuildRequest_ex(reqType, domainName, path, pathLen, reqSz, contentType, "", buf, bufSize);
|
||||
}
|
||||
|
||||
/* Returns 1 if the buffer contains a CR or LF byte, 0 otherwise. Used to
|
||||
* reject attacker-controlled URL components that would allow HTTP header
|
||||
* injection or request splitting. */
|
||||
static int wolfIO_UrlHasCrlf(const char* in, int inSz)
|
||||
{
|
||||
int i = 0;
|
||||
if (in == NULL)
|
||||
return 0;
|
||||
for (i = 0; i < inSz; i++) {
|
||||
if (in[i] == '\r' || in[i] == '\n')
|
||||
return 1;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
int wolfIO_HttpBuildRequest_ex(const char *reqType, const char *domainName,
|
||||
const char *path, int pathLen, int reqSz, const char *contentType,
|
||||
const char *exHdrs, byte *buf, int bufSize)
|
||||
@@ -2082,11 +2103,24 @@ int wolfIO_HttpBuildRequest_ex(const char *reqType, const char *domainName,
|
||||
word32 blankStrLen, http11StrLen, hostStrLen, contentLenStrLen,
|
||||
contentTypeStrLen, singleCrLfStrLen, doubleCrLfStrLen;
|
||||
|
||||
/* reject NULL pointers and invalid lengths before any deref or length
|
||||
* math: the XSTRLEN/copy calls below assume non-NULL inputs */
|
||||
if (reqType == NULL || domainName == NULL || path == NULL ||
|
||||
contentType == NULL || pathLen < 0 || bufSize <= 0)
|
||||
return 0;
|
||||
|
||||
reqTypeLen = (word32)XSTRLEN(reqType);
|
||||
domainNameLen = (word32)XSTRLEN(domainName);
|
||||
reqSzStrLen = wolfIO_Word16ToString(reqSzStr, (word16)reqSz);
|
||||
contentTypeLen = (word32)XSTRLEN(contentType);
|
||||
|
||||
/* reject CR/LF in the path or host to prevent HTTP header injection or
|
||||
* request splitting from attacker-controlled URL components */
|
||||
if (wolfIO_UrlHasCrlf(path, pathLen) ||
|
||||
wolfIO_UrlHasCrlf(domainName, (int)domainNameLen)) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
blankStrLen = (word32)XSTRLEN(blankStr);
|
||||
http11StrLen = (word32)XSTRLEN(http11Str);
|
||||
hostStrLen = (word32)XSTRLEN(hostStr);
|
||||
@@ -2126,7 +2160,10 @@ int wolfIO_HttpBuildRequest_ex(const char *reqType, const char *domainName,
|
||||
buf += reqTypeLen; bufSize -= (int)reqTypeLen;
|
||||
XSTRNCPY((char*)buf, blankStr, (size_t)bufSize);
|
||||
buf += blankStrLen; bufSize -= (int)blankStrLen;
|
||||
XSTRNCPY((char*)buf, path, (size_t)bufSize);
|
||||
/* path may not be NUL terminated (CRL raw-URL case passes a pointer into
|
||||
* the certificate with only pathLen valid bytes), so bound the copy to
|
||||
* pathLen rather than scanning for a NUL */
|
||||
XMEMCPY((char*)buf, path, (size_t)pathLen);
|
||||
buf += pathLen; bufSize -= (int)pathLen;
|
||||
XSTRNCPY((char*)buf, http11Str, (size_t)bufSize);
|
||||
buf += http11StrLen; bufSize -= (int)http11StrLen;
|
||||
@@ -2251,8 +2288,11 @@ int EmbedOcspLookup(void* ctx, const char* url, int urlSz,
|
||||
httpBufSz = wolfIO_HttpBuildRequestOcsp(domainName, path, ocspReqSz,
|
||||
httpBuf, httpBufSz);
|
||||
|
||||
ret = wolfIO_TcpConnect(&sfd, domainName, port, io_timeout_sec);
|
||||
if (ret != 0) {
|
||||
if (httpBufSz <= 0) {
|
||||
WOLFSSL_MSG("Unable to build OCSP request");
|
||||
}
|
||||
else if ((ret = wolfIO_TcpConnect(&sfd, domainName, port,
|
||||
io_timeout_sec)) != 0) {
|
||||
WOLFSSL_MSG("OCSP Responder connection failed");
|
||||
}
|
||||
else if (wolfIO_Send(sfd, (char*)httpBuf, httpBufSz, 0) !=
|
||||
@@ -2351,8 +2391,11 @@ int EmbedCrlLookup(WOLFSSL_CRL* crl, const char* url, int urlSz)
|
||||
httpBufSz = wolfIO_HttpBuildRequestCrl(url, urlSz, domainName,
|
||||
httpBuf, httpBufSz);
|
||||
|
||||
ret = wolfIO_TcpConnect(&sfd, domainName, port, io_timeout_sec);
|
||||
if (ret != 0) {
|
||||
if (httpBufSz <= 0) {
|
||||
WOLFSSL_MSG("Unable to build CRL request");
|
||||
}
|
||||
else if ((ret = wolfIO_TcpConnect(&sfd, domainName, port,
|
||||
io_timeout_sec)) != 0) {
|
||||
WOLFSSL_MSG("CRL connection failed");
|
||||
}
|
||||
else if (wolfIO_Send(sfd, (char*)httpBuf, httpBufSz, 0)
|
||||
|
||||
@@ -35294,6 +35294,7 @@ TEST_CASE testCases[] = {
|
||||
TEST_DECL(test_ocsp_cert_unknown_crl_fallback_nonleaf),
|
||||
TEST_DECL(test_tls13_nonblock_ocsp_low_mfl),
|
||||
TEST_DECL(test_ocsp_responder),
|
||||
TEST_DECL(test_wolfIO_DecodeUrl_crlf_reject),
|
||||
TEST_TLS_DECLS,
|
||||
TEST_SESSION_DECLS,
|
||||
TEST_DECL(test_wc_DhSetNamedKey),
|
||||
|
||||
@@ -1809,3 +1809,101 @@ int test_ocsp_responder(void)
|
||||
return TEST_SKIPPED;
|
||||
}
|
||||
#endif /* HAVE_OCSP_RESPONDER && !NO_SHA && !NO_RSA */
|
||||
|
||||
#if defined(HAVE_HTTP_CLIENT)
|
||||
/* A peer-supplied AIA/CRL URL must not be able to smuggle CR/LF into the
|
||||
* outbound OCSP/CRL HTTP request (header injection / request splitting). */
|
||||
int test_wolfIO_DecodeUrl_crlf_reject(void)
|
||||
{
|
||||
EXPECT_DECLS;
|
||||
char domainName[80];
|
||||
char path[80];
|
||||
word16 port;
|
||||
byte reqBuf[512];
|
||||
char* tightPath = NULL;
|
||||
int tightLen = 16;
|
||||
const char* crlfInPath = "http://ocsp.example.com/ocsp\r\nX-Injected: 1";
|
||||
const char* crlfInHost = "http://evil\r\nHost: x/ocsp";
|
||||
const char* crlfInV6Host = "http://[::1\r\n]/ocsp";
|
||||
const char* cleanUrl = "http://ocsp.example.com/ocsp";
|
||||
const char* taintedPath = "/ocsp\r\nX-Injected: 1";
|
||||
const char* crlfAfterCap =
|
||||
"http://ocsp.example.com/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\r\nX: 1";
|
||||
|
||||
/* parser rejects CR/LF in the path */
|
||||
ExpectIntLT(wolfIO_DecodeUrl(crlfInPath, (int)XSTRLEN(crlfInPath),
|
||||
domainName, path, &port), 0);
|
||||
|
||||
/* parser rejects CR/LF in the host */
|
||||
ExpectIntLT(wolfIO_DecodeUrl(crlfInHost, (int)XSTRLEN(crlfInHost),
|
||||
domainName, path, &port), 0);
|
||||
|
||||
/* parser rejects CR/LF in an IPv6 bracket-literal host */
|
||||
ExpectIntLT(wolfIO_DecodeUrl(crlfInV6Host, (int)XSTRLEN(crlfInV6Host),
|
||||
domainName, path, &port), 0);
|
||||
|
||||
/* sink rejects a tainted path even when the parser is bypassed, which is
|
||||
* the CRL raw-URL case */
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("POST", "ocsp.example.com",
|
||||
taintedPath, (int)XSTRLEN(taintedPath), 0, "application/ocsp-request",
|
||||
reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
|
||||
/* sink rejects CR/LF even when it appears beyond the decoder cap */
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", "ocsp.example.com",
|
||||
crlfAfterCap, (int)XSTRLEN(crlfAfterCap), 0, "", reqBuf,
|
||||
(int)sizeof(reqBuf)), 0);
|
||||
|
||||
/* sink rejects CR/LF in the host (domainName) with a clean path */
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("POST", "evil.example.com\r\nX: 1",
|
||||
"/ocsp", (int)XSTRLEN("/ocsp"), 0, "application/ocsp-request", reqBuf,
|
||||
(int)sizeof(reqBuf)), 0);
|
||||
|
||||
/* the CRL raw-URL path is not NUL terminated, so the request builder must
|
||||
* copy only pathLen bytes. Allocate exactly pathLen bytes with no
|
||||
* terminator so an over-read past the buffer is caught under ASAN. */
|
||||
tightPath = (char*)XMALLOC((size_t)tightLen, NULL, DYNAMIC_TYPE_TMP_BUFFER);
|
||||
ExpectNotNull(tightPath);
|
||||
if (tightPath != NULL) {
|
||||
XMEMSET(tightPath, 'a', (size_t)tightLen);
|
||||
tightPath[0] = '/';
|
||||
ExpectIntGT(wolfIO_HttpBuildRequest("GET", "ocsp.example.com",
|
||||
tightPath, tightLen, 0, "", reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
}
|
||||
|
||||
/* sink rejects a negative pathLen before any length math or copy: with the
|
||||
* old XMEMCPY this wrapped (word32)pathLen and overran the output buffer */
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", "ocsp.example.com", "/ocsp",
|
||||
-1, 0, "", reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
|
||||
/* sink rejects a non-positive bufSize before any copy */
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", "ocsp.example.com", "/ocsp",
|
||||
(int)XSTRLEN("/ocsp"), 0, "", reqBuf, 0), 0);
|
||||
|
||||
/* sink rejects NULL pointer parameters at the boundary, before any
|
||||
* XSTRLEN deref */
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest(NULL, "ocsp.example.com", "/ocsp",
|
||||
(int)XSTRLEN("/ocsp"), 0, "", reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", NULL, "/ocsp",
|
||||
(int)XSTRLEN("/ocsp"), 0, "", reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", "ocsp.example.com", NULL,
|
||||
0, 0, "", reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
ExpectIntEQ(wolfIO_HttpBuildRequest("GET", "ocsp.example.com", "/ocsp",
|
||||
(int)XSTRLEN("/ocsp"), 0, NULL, reqBuf, (int)sizeof(reqBuf)), 0);
|
||||
|
||||
/* positive control: a clean URL decodes and still builds a request */
|
||||
ExpectIntEQ(wolfIO_DecodeUrl(cleanUrl, (int)XSTRLEN(cleanUrl), domainName,
|
||||
path, &port), 0);
|
||||
ExpectIntGT(wolfIO_HttpBuildRequest("POST", domainName, path,
|
||||
(int)XSTRLEN(path), 0, "application/ocsp-request", reqBuf,
|
||||
(int)sizeof(reqBuf)), 0);
|
||||
|
||||
XFREE(tightPath, NULL, DYNAMIC_TYPE_TMP_BUFFER);
|
||||
|
||||
return EXPECT_RESULT();
|
||||
}
|
||||
#else
|
||||
int test_wolfIO_DecodeUrl_crlf_reject(void)
|
||||
{
|
||||
return TEST_SKIPPED;
|
||||
}
|
||||
#endif /* HAVE_HTTP_CLIENT */
|
||||
|
||||
@@ -35,5 +35,6 @@ int test_ocsp_cert_unknown_crl_fallback_nonleaf(void);
|
||||
int test_tls13_nonblock_ocsp_low_mfl(void);
|
||||
int test_ocsp_responder(void);
|
||||
int test_ocsp_ancestor_responder_rejected(void);
|
||||
int test_wolfIO_DecodeUrl_crlf_reject(void);
|
||||
#endif /* WOLFSSL_TEST_OCSP_H */
|
||||
|
||||
|
||||
Reference in New Issue
Block a user