packages/squid
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!39 fix CVE-2021-28651 CVE-2021-28652 CVE-2021-28662 CVE-2021-31806 CVE-2021-31808 CVE-2021-33620

Browse Source 
From: @haochenstar
Reviewed-by: @wangxp006
Signed-off-by: @wangxp006
This commit is contained in:
openeuler-ci-bot 2021-06-16 09:04:51 +00:00 committed by Gitee
commit 5db7d5ba16
7 changed files with 1230 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

195
backport-0001-CVE-2021-28652.patch Normal file
View File

@ -0,0 +1,195 @@
From 417da4006cf5c97d44e74431b816fc58fec9e270 Mon Sep 17 00:00:00 2001
From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Mon, 18 Mar 2019 17:48:21 +0000
Subject: [PATCH] Fix incremental parsing of chunked quoted extensions (#310)
Before this change, incremental parsing of quoted chunked extensions
was broken for two reasons:
* Http::One::Parser::skipLineTerminator() unexpectedly threw after
partially received quoted chunk extension value.
* When Http::One::Tokenizer was unable to parse a quoted extension,
it incorrectly restored the input buffer to the beginning of the
extension value (instead of the extension itself), thus making
further incremental parsing iterations impossible.
IMO, the reason for this problem was that Http::One::Tokenizer::qdText()
could not distinguish two cases (returning false in both):
* the end of the quoted string not yet reached
* an input error, e.g., wrong/unexpected character
A possible approach could be to improve Http::One::Tokenizer, making it
aware about "needs more data" state. However, to be acceptable,
these improvements should be done in the base Parser::Tokenizer
class instead. These changes seem to be non-trivial and could be
done separately and later.
Another approach, used here, is to simplify the complex and error-prone
chunked extensions parsing algorithm, fixing incremental parsing bugs
and still parse incrementally in almost all cases. The performance
regression could be expected only in relatively rare cases of partially
received or malformed extensions.
Also:
* fixed parsing of partial use-original-body extension values
* do not treat an invalid use-original-body as an unknown extension
* optimization: parse use-original-body extension only in ICAP context
(i.e., where it is expected)
* improvement: added a new API to TeChunkedParser to specify known
chunked extensions list
---
src/Debug.h | 4 ++++
src/parser/Makefile.am | 1 +
src/parser/Tokenizer.cc | 40 ++++++++++++++++++++++++++++++++++++++++
src/parser/Tokenizer.h | 13 +++++++++++++
src/parser/forward.h | 22 ++++++++++++++++++++++
5 files changed, 80 insertions(+)
create mode 100644 src/parser/forward.h
diff --git a/src/Debug.h b/src/Debug.h
index 7fb1ed5..8c24175 100644
--- a/src/Debug.h
+++ b/src/Debug.h
@@ -99,6 +99,10 @@ public:
/// configures the active debugging context to write syslog ALERT
static void ForceAlert();
+
+ /// prefixes each grouped debugs() line after the first one in the group
+ static std::ostream& Extra(std::ostream &os) { return os << "\n "; }
+
private:
static Context *Current; ///< deepest active context; nil outside debugs()
};
diff --git a/src/parser/Makefile.am b/src/parser/Makefile.am
index aef3235..c08d1d5 100644
--- a/src/parser/Makefile.am
+++ b/src/parser/Makefile.am
@@ -13,6 +13,7 @@ noinst_LTLIBRARIES = libparser.la
libparser_la_SOURCES = \
BinaryTokenizer.h \
BinaryTokenizer.cc \
+ forward.h \
Tokenizer.h \
Tokenizer.cc
diff --git a/src/parser/Tokenizer.cc b/src/parser/Tokenizer.cc
index 99f8eb3..0b44e40 100644
--- a/src/parser/Tokenizer.cc
+++ b/src/parser/Tokenizer.cc
@@ -10,7 +10,9 @@
#include "squid.h"
#include "Debug.h"
+#include "parser/forward.h"
#include "parser/Tokenizer.h"
+#include "sbuf/Stream.h"
#include <cerrno>
#if HAVE_CTYPE_H
@@ -96,6 +98,23 @@ Parser::Tokenizer::prefix(SBuf &returnedToken, const CharacterSet &tokenChars, c
return true;
}
+SBuf
+Parser::Tokenizer::prefix(const char *description, const CharacterSet &tokenChars, const SBuf::size_type limit)
+{
+ if (atEnd())
+ throw InsufficientInput();
+
+ SBuf result;
+
+ if (!prefix(result, tokenChars, limit))
+ throw TexcHere(ToSBuf("cannot parse ", description));
+
+ if (atEnd())
+ throw InsufficientInput();
+
+ return result;
+}
+
bool
Parser::Tokenizer::suffix(SBuf &returnedToken, const CharacterSet &tokenChars, const SBuf::size_type limit)
{
@@ -283,3 +302,24 @@ Parser::Tokenizer::int64(int64_t & result, int base, bool allowSign, const SBuf:
return success(s - range.rawContent());
}
+int64_t
+Parser::Tokenizer::udec64(const char *description, const SBuf::size_type limit)
+{
+ if (atEnd())
+ throw InsufficientInput();
+
+ int64_t result = 0;
+
+ // Since we only support unsigned decimals, a parsing failure with a
+ // non-empty input always implies invalid/malformed input (or a buggy
+ // limit=0 caller). TODO: Support signed and non-decimal integers by
+ // refactoring int64() to detect insufficient input.
+ if (!int64(result, 10, false, limit))
+ throw TexcHere(ToSBuf("cannot parse ", description));
+
+ if (atEnd())
+ throw InsufficientInput(); // more digits may be coming
+
+ return result;
+}
+
diff --git a/src/parser/Tokenizer.h b/src/parser/Tokenizer.h
index f04fd3e..6ae8162 100644
--- a/src/parser/Tokenizer.h
+++ b/src/parser/Tokenizer.h
@@ -143,6 +143,19 @@ public:
*/
bool int64(int64_t &result, int base = 0, bool allowSign = true, SBuf::size_type limit = SBuf::npos);
+ /*
+ * The methods below mimic their counterparts documented above, but they
+ * throw on errors, including InsufficientInput. The field description
+ * parameter is used for error reporting and debugging.
+ */
+
+ /// prefix() wrapper but throws InsufficientInput if input contains
+ /// nothing but the prefix (i.e. if the prefix is not "terminated")
+ SBuf prefix(const char *description, const CharacterSet &tokenChars, SBuf::size_type limit = SBuf::npos);
+
+ /// int64() wrapper but limited to unsigned decimal integers (for now)
+ int64_t udec64(const char *description, SBuf::size_type limit = SBuf::npos);
+
protected:
SBuf consume(const SBuf::size_type n);
SBuf::size_type success(const SBuf::size_type n);
diff --git a/src/parser/forward.h b/src/parser/forward.h
new file mode 100644
index 0000000..5a95b7a
--- /dev/null
+++ b/src/parser/forward.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 1996-2019 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#ifndef SQUID_PARSER_FORWARD_H
+#define SQUID_PARSER_FORWARD_H
+
+namespace Parser {
+class Tokenizer;
+class BinaryTokenizer;
+
+// TODO: Move this declaration (to parser/Elements.h) if we need more like it.
+/// thrown by modern "incremental" parsers when they need more data
+class InsufficientInput {};
+} // namespace Parser
+
+#endif /* SQUID_PARSER_FORWARD_H */
+
--
2.23.0

631
backport-0002-CVE-2021-28652.patch Normal file
View File

@ -0,0 +1,631 @@
commit 0003e3518dc95e4b5ab46b5140af79b22253048e
Author: Amos Jeffries <yadij@users.noreply.github.com>
Date: 2021-04-30 05:15:44 +0000
Bug 5106: Broken cache manager URL parsing (#788)
Use already parsed request-target URL in cache manager and
update CacheManager to Tokanizer based URL parse
Removing use of sscan() and regex string processing which have
proven to be problematic on many levels. Most particularly with
regards to tolerance of normally harmless garbage syntax in URLs
received.
Support for generic URI schemes is added possibly resolving some
issues reported with ftp:// URL and manager access via ftp_port
sockets.
Truly generic support for /squid-internal-mgr/ path prefix is
added, fixing some user confusion about its use on cache_object:
scheme URLs.
TODO: support for single-name parameters and URL #fragments
are left to future updates. As is refactoring the QueryParams
data storage to avoid SBuf data copying.
diff --git a/src/CacheManager.h b/src/CacheManager.h
index 78a69f799..74705c58a 100644
--- a/src/CacheManager.h
+++ b/src/CacheManager.h
@@ -9,6 +9,7 @@
#ifndef SQUID_CACHEMANAGER_H
#define SQUID_CACHEMANAGER_H
+#include "anyp/forward.h"
#include "comm/forward.h"
#include "mgr/Action.h"
#include "mgr/ActionProfile.h"
@@ -50,7 +51,7 @@ public:
protected:
CacheManager() {} ///< use Instance() instead
- Mgr::CommandPointer ParseUrl(const char *url);
+ Mgr::CommandPointer ParseUrl(const AnyP::Uri &);
void ParseHeaders(const HttpRequest * request, Mgr::ActionParams &params);
int CheckPassword(const Mgr::Command &cmd);
char *PasswdGet(Mgr::ActionPasswordList *, const char *);
diff --git a/src/cache_manager.cc b/src/cache_manager.cc
index 9fe9bbb89..8055ece6b 100644
--- a/src/cache_manager.cc
+++ b/src/cache_manager.cc
@@ -26,7 +26,9 @@
#include "mgr/Forwarder.h"
#include "mgr/FunAction.h"
#include "mgr/QueryParams.h"
+#include "parser/Tokenizer.h"
#include "protos.h"
+#include "sbuf/Stream.h"
#include "sbuf/StringConvert.h"
#include "SquidConfig.h"
#include "SquidTime.h"
@@ -147,82 +149,87 @@ CacheManager::createRequestedAction(const Mgr::ActionParams &params)
return cmd->profile->creator->create(cmd);
}
+static const CharacterSet &
+MgrFieldChars(const AnyP::ProtocolType &protocol)
+{
+ // Deprecated cache_object:// scheme used '@' to delimit passwords
+ if (protocol == AnyP::PROTO_CACHE_OBJECT) {
+ static const CharacterSet fieldChars = CharacterSet("cache-object-field", "@?#").complement();
+ return fieldChars;
+ }
+
+ static const CharacterSet actionChars = CharacterSet("mgr-field", "?#").complement();
+ return actionChars;
+}
+
/**
- \ingroup CacheManagerInternal
* define whether the URL is a cache-manager URL and parse the action
* requested by the user. Checks via CacheManager::ActionProtection() that the
* item is accessible by the user.
- \retval CacheManager::cachemgrStateData state object for the following handling
- \retval NULL if the action can't be found or can't be accessed by the user
+ *
+ * Syntax:
+ *
+ * scheme "://" authority [ '/squid-internal-mgr' ] path-absolute [ '@' unreserved ] '?' query-string
+ *
+ * see RFC 3986 for definitions of scheme, authority, path-absolute, query-string
+ *
+ * \returns Mgr::Command object with action to perform and parameters it might use
*/
Mgr::Command::Pointer
-CacheManager::ParseUrl(const char *url)
+CacheManager::ParseUrl(const AnyP::Uri &uri)
{
- int t;
- LOCAL_ARRAY(char, host, MAX_URL);
- LOCAL_ARRAY(char, request, MAX_URL);
- LOCAL_ARRAY(char, password, MAX_URL);
- LOCAL_ARRAY(char, params, MAX_URL);
- host[0] = 0;
- request[0] = 0;
- password[0] = 0;
- params[0] = 0;
- int pos = -1;
- int len = strlen(url);
- Must(len > 0);
- t = sscanf(url, "cache_object://%[^/]/%[^@?]%n@%[^?]?%s", host, request, &pos, password, params);
- if (t < 3) {
- t = sscanf(url, "cache_object://%[^/]/%[^?]%n?%s", host, request, &pos, params);
- }
- if (t < 1) {
- t = sscanf(url, "http://%[^/]/squid-internal-mgr/%[^?]%n?%s", host, request, &pos, params);
- }
- if (t < 1) {
- t = sscanf(url, "https://%[^/]/squid-internal-mgr/%[^?]%n?%s", host, request, &pos, params);
- }
- if (t < 2) {
- if (strncmp("cache_object://",url,15)==0)
- xstrncpy(request, "menu", MAX_URL);
- else
- xstrncpy(request, "index", MAX_URL);
- }
+ Parser::Tokenizer tok(uri.path());
-#if _SQUID_OS2_
- if (t == 2 && request[0] == '\0') {
- /*
- * emx's sscanf insists of returning 2 because it sets request
- * to null
- */
- if (strncmp("cache_object://",url,15)==0)
- xstrncpy(request, "menu", MAX_URL);
- else
- xstrncpy(request, "index", MAX_URL);
- }
-#endif
+ static const SBuf internalMagicPrefix("/squid-internal-mgr/");
+ if (!tok.skip(internalMagicPrefix) && !tok.skip('/'))
+ throw TextException("invalid URL path", Here());
- debugs(16, 3, HERE << "MGR request: t=" << t << ", host='" << host << "', request='" << request << "', pos=" << pos <<
- ", password='" << password << "', params='" << params << "'");
+ Mgr::Command::Pointer cmd = new Mgr::Command();
+ cmd->params.httpUri = SBufToString(uri.absolute());
- Mgr::ActionProfile::Pointer profile = findAction(request);
- if (!profile) {
- debugs(16, DBG_IMPORTANT, "CacheManager::ParseUrl: action '" << request << "' not found");
- return NULL;
+ const auto &fieldChars = MgrFieldChars(uri.getScheme());
+
+ SBuf action;
+ if (!tok.prefix(action, fieldChars)) {
+ if (uri.getScheme() == AnyP::PROTO_CACHE_OBJECT) {
+ static const SBuf menuReport("menu");
+ action = menuReport;
+ } else {
+ static const SBuf indexReport("index");
+ action = indexReport;
+ }
}
+ cmd->params.actionName = SBufToString(action);
+
+ const auto profile = findAction(action.c_str());
+ if (!profile)
+ throw TextException(ToSBuf("action '", action, "' not found"), Here());
const char *prot = ActionProtection(profile);
- if (!strcmp(prot, "disabled") || !strcmp(prot, "hidden")) {
- debugs(16, DBG_IMPORTANT, "CacheManager::ParseUrl: action '" << request << "' is " << prot);
- return NULL;
+ if (!strcmp(prot, "disabled") || !strcmp(prot, "hidden"))
+ throw TextException(ToSBuf("action '", action, "' is ", prot), Here());
+ cmd->profile = profile;
+
+ SBuf passwd;
+ if (uri.getScheme() == AnyP::PROTO_CACHE_OBJECT && tok.skip('@')) {
+ (void)tok.prefix(passwd, fieldChars);
+ cmd->params.password = SBufToString(passwd);
}
- Mgr::Command::Pointer cmd = new Mgr::Command;
- if (!Mgr::QueryParams::Parse(params, cmd->params.queryParams))
- return NULL;
- cmd->profile = profile;
- cmd->params.httpUri = url;
- cmd->params.userName = String();
- cmd->params.password = password;
- cmd->params.actionName = request;
+ // TODO: fix when AnyP::Uri::parse() separates path?query#fragment
+ SBuf params;
+ if (tok.skip('?')) {
+ params = tok.remaining();
+ Mgr::QueryParams::Parse(tok, cmd->params.queryParams);
+ }
+
+ if (!tok.skip('#') && !tok.atEnd())
+ throw TextException("invalid characters in URL", Here());
+ // else ignore #fragment (if any)
+
+ debugs(16, 3, "MGR request: host=" << uri.host() << ", action=" << action <<
+ ", password=" << passwd << ", params=" << params);
+
return cmd;
}
@@ -305,11 +312,15 @@ CacheManager::CheckPassword(const Mgr::Command &cmd)
void
CacheManager::Start(const Comm::ConnectionPointer &client, HttpRequest * request, StoreEntry * entry)
{
- debugs(16, 3, "CacheManager::Start: '" << entry->url() << "'" );
+ debugs(16, 3, "request-url= '" << request->url << "', entry-url='" << entry->url() << "'");
- Mgr::Command::Pointer cmd = ParseUrl(entry->url());
- if (!cmd) {
- ErrorState *err = new ErrorState(ERR_INVALID_URL, Http::scNotFound, request);
+ Mgr::Command::Pointer cmd;
+ try {
+ cmd = ParseUrl(request->url);
+
+ } catch (...) {
+ debugs(16, 2, "request URL error: " << CurrentException);
+ const auto err = new ErrorState(ERR_INVALID_URL, Http::scNotFound, request);
err->url = xstrdup(entry->url());
errorAppendEntry(entry, err);
entry->expires = squid_curtime;
@@ -473,4 +484,3 @@ CacheManager::GetInstance()
}
return instance;
}
-
diff --git a/src/mgr/QueryParams.cc b/src/mgr/QueryParams.cc
index 831694245..a53dee1c7 100644
--- a/src/mgr/QueryParams.cc
+++ b/src/mgr/QueryParams.cc
@@ -14,6 +14,10 @@
#include "mgr/IntParam.h"
#include "mgr/QueryParams.h"
#include "mgr/StringParam.h"
+#include "parser/Tokenizer.h"
+#include "sbuf/StringConvert.h"
+
+#include <limits>
Mgr::QueryParam::Pointer
Mgr::QueryParams::get(const String& name) const
@@ -65,61 +69,76 @@ Mgr::QueryParams::find(const String& name) const
return iter;
}
-bool
-Mgr::QueryParams::ParseParam(const String& paramStr, Param& param)
+/**
+ * Parses the value part of a "param=value" URL section.
+ * Value can be a comma-separated list of integers or an opaque string.
+ *
+ * value = *pchar | ( 1*DIGIT *( ',' 1*DIGIT ) )
+ *
+ * \note opaque string may be a list with a non-integer (e.g., "1,2,3,z")
+ */
+Mgr::QueryParam::Pointer
+ParseParamValue(const SBuf &rawValue)
{
- bool parsed = false;
- regmatch_t pmatch[3];
- regex_t intExpr;
- regcomp(&intExpr, "^([a-z][a-z0-9_]*)=([0-9]+((,[0-9]+))*)$", REG_EXTENDED | REG_ICASE);
- regex_t stringExpr;
- regcomp(&stringExpr, "^([a-z][a-z0-9_]*)=([^&= ]+)$", REG_EXTENDED | REG_ICASE);
- if (regexec(&intExpr, paramStr.termedBuf(), 3, pmatch, 0) == 0) {
- param.first = paramStr.substr(pmatch[1].rm_so, pmatch[1].rm_eo);
- std::vector<int> array;
- int n = pmatch[2].rm_so;
- for (int i = n; i < pmatch[2].rm_eo; ++i) {
- if (paramStr[i] == ',') {
- array.push_back(atoi(paramStr.substr(n, i).termedBuf()));
- n = i + 1;
- }
- }
- if (n < pmatch[2].rm_eo)
- array.push_back(atoi(paramStr.substr(n, pmatch[2].rm_eo).termedBuf()));
- param.second = new IntParam(array);
- parsed = true;
- } else if (regexec(&stringExpr, paramStr.termedBuf(), 3, pmatch, 0) == 0) {
- param.first = paramStr.substr(pmatch[1].rm_so, pmatch[1].rm_eo);
- param.second = new StringParam(paramStr.substr(pmatch[2].rm_so, pmatch[2].rm_eo));
- parsed = true;
+ static const CharacterSet comma("comma", ",");
+
+ Parser::Tokenizer tok(rawValue);
+ std::vector<int> array;
+ int64_t intVal = 0;
+ while (tok.int64(intVal, 10, false)) {
+ Must(intVal >= std::numeric_limits<int>::min());
+ Must(intVal <= std::numeric_limits<int>::max());
+ array.emplace_back(intVal);
+ // integer list has comma between values.
+ // Require at least one potential DIGIT after the skipped ','
+ if (tok.remaining().length() > 1)
+ (void)tok.skipOne(comma);
}
- regfree(&stringExpr);
- regfree(&intExpr);
- return parsed;
+
+ if (tok.atEnd())
+ return new Mgr::IntParam(array);
+ else
+ return new Mgr::StringParam(SBufToString(rawValue));
}
-bool
-Mgr::QueryParams::Parse(const String& aParamsStr, QueryParams& aParams)
+/**
+ * Syntax:
+ * query = [ param *( '&' param ) ]
+ * param = name '=' value
+ * name = [a-zA-Z0-9]+
+ * value = *pchar | ( 1*DIGIT *( ',' 1*DIGIT ) )
+ */
+void
+Mgr::QueryParams::Parse(Parser::Tokenizer &tok, QueryParams &aParams)
{
- if (aParamsStr.size() != 0) {
- Param param;
- size_t n = 0;
- size_t len = aParamsStr.size();
- for (size_t i = n; i < len; ++i) {
- if (aParamsStr[i] == '&') {
- if (!ParseParam(aParamsStr.substr(n, i), param))
- return false;
- aParams.params.push_back(param);
- n = i + 1;
- }
- }
- if (n < len) {
- if (!ParseParam(aParamsStr.substr(n, len), param))
- return false;
- aParams.params.push_back(param);
- }
+ static const CharacterSet nameChars = CharacterSet("param-name", "_") + CharacterSet::ALPHA + CharacterSet::DIGIT;
+ static const CharacterSet valueChars = CharacterSet("param-value", "&= #").complement();
+ static const CharacterSet delimChars("param-delim", "&");
+
+ while (!tok.atEnd()) {
+
+ // TODO: remove '#' processing when AnyP::Uri splits 'query#fragment' properly
+ // #fragment handled by caller. Do not throw.
+ if (tok.remaining()[0] == '#')
+ return;
+
+ if (tok.skipAll(delimChars))
+ continue;
+
+ SBuf nameStr;
+ if (!tok.prefix(nameStr, nameChars))
+ throw TextException("invalid query parameter name", Here());
+ if (!tok.skip('='))
+ throw TextException("missing parameter value", Here());
+
+ SBuf valueStr;
+ if (!tok.prefix(valueStr, valueChars))
+ throw TextException("missing or malformed parameter value", Here());
+
+ const auto name = SBufToString(nameStr);
+ const auto value = ParseParamValue(valueStr);
+ aParams.params.emplace_back(name, value);
}
- return true;
}
Mgr::QueryParam::Pointer
@@ -138,4 +157,3 @@ Mgr::QueryParams::CreateParam(QueryParam::Type aType)
}
return NULL;
}
-
diff --git a/src/mgr/QueryParams.h b/src/mgr/QueryParams.h
index bb8f40308..450c20f86 100644
--- a/src/mgr/QueryParams.h
+++ b/src/mgr/QueryParams.h
@@ -13,9 +13,11 @@
#include "ipc/forward.h"
#include "mgr/QueryParam.h"
+#include "parser/Tokenizer.h"
#include "SquidString.h"
-#include <vector>
+
#include <utility>
+#include <vector>
namespace Mgr
{
@@ -32,15 +34,13 @@ public:
void pack(Ipc::TypedMsgHdr& msg) const; ///< store params into msg
void unpack(const Ipc::TypedMsgHdr& msg); ///< load params from msg
/// parses the query string parameters
- static bool Parse(const String& aParamsStr, QueryParams& aParams);
+ static void Parse(Parser::Tokenizer &, QueryParams &);
private:
/// find query parameter by name
Params::const_iterator find(const String& name) const;
/// creates a parameter of the specified type
static QueryParam::Pointer CreateParam(QueryParam::Type aType);
- /// parses string like "param=value"; returns true if success
- static bool ParseParam(const String& paramStr, Param& param);
private:
Params params;
diff --git a/src/tests/stub_libmgr.cc b/src/tests/stub_libmgr.cc
index f8be88a58..cd3ffc2de 100644
--- a/src/tests/stub_libmgr.cc
+++ b/src/tests/stub_libmgr.cc
@@ -174,11 +174,10 @@ void Mgr::IoAction::dump(StoreEntry* entry) STUB
Mgr::QueryParam::Pointer Mgr::QueryParams::get(const String& name) const STUB_RETVAL(Mgr::QueryParam::Pointer(NULL))
void Mgr::QueryParams::pack(Ipc::TypedMsgHdr& msg) const STUB
void Mgr::QueryParams::unpack(const Ipc::TypedMsgHdr& msg) STUB
-bool Mgr::QueryParams::Parse(const String& aParamsStr, QueryParams& aParams) STUB_RETVAL(false)
+void Mgr::QueryParams::Parse(Parser::Tokenizer &, QueryParams &) STUB
//private:
//Params::const_iterator Mgr::QueryParams::find(const String& name) const STUB_RETVAL(new Mgr::Params::const_iterator(*this))
Mgr::QueryParam::Pointer Mgr::QueryParams::CreateParam(QueryParam::Type aType) STUB_RETVAL(Mgr::QueryParam::Pointer(NULL))
-bool Mgr::QueryParams::ParseParam(const String& paramStr, Param& param) STUB_RETVAL(false)
#include "mgr/Registration.h"
//void Mgr::RegisterAction(char const * action, char const * desc, OBJH * handler, int pw_req_flag, int atomic);
diff --git a/src/tests/testCacheManager.cc b/src/tests/testCacheManager.cc
index f02396176..7d6631aae 100644
--- a/src/tests/testCacheManager.cc
+++ b/src/tests/testCacheManager.cc
@@ -7,6 +7,7 @@
*/
#include "squid.h"
+#include "anyp/Uri.h"
#include "CacheManager.h"
#include "mgr/Action.h"
#include "Store.h"
@@ -17,11 +18,19 @@
CPPUNIT_TEST_SUITE_REGISTRATION( testCacheManager );
+/// Provides test code access to CacheManager internal symbols
+class CacheManagerInternals : public CacheManager
+{
+public:
+ void ParseUrl(const AnyP::Uri &u) { CacheManager::ParseUrl(u); }
+};
+
/* init memory pools */
void testCacheManager::setUp()
{
Mem::Init();
+ AnyP::UriScheme::Init();
}
/*
@@ -66,3 +75,146 @@ testCacheManager::testRegister()
CPPUNIT_ASSERT_EQUAL(1,(int)sentry->flags);
}
+void
+testCacheManager::testParseUrl()
+{
+ auto *mgr = static_cast<CacheManagerInternals *>(CacheManager::GetInstance());
+ CPPUNIT_ASSERT(mgr != nullptr);
+
+ std::vector<AnyP::ProtocolType> validSchemes = {
+ AnyP::PROTO_CACHE_OBJECT,
+ AnyP::PROTO_HTTP,
+ AnyP::PROTO_HTTPS,
+ AnyP::PROTO_FTP
+ };
+
+ AnyP::Uri mgrUrl;
+ mgrUrl.host("localhost");
+ mgrUrl.port(3128);
+
+ const std::vector<const char *> magicPrefixes = {
+ "/",
+ "/squid-internal-mgr/"
+ };
+
+ const std::vector<const char *> validActions = {
+ "",
+ "menu"
+ };
+
+ const std::vector<const char *> invalidActions = {
+ "INVALID" // any unregistered name
+ };
+
+ const std::vector<const char *> validParams = {
+ "",
+ "?",
+ "?&",
+ "?&&&&&&&&&&&&",
+ "?foo=bar",
+ "?0123456789=bar",
+ "?foo=bar&",
+ "?foo=bar&&&&",
+ "?&foo=bar",
+ "?&&&&foo=bar",
+ "?&foo=bar&",
+ "?&&&&foo=bar&&&&",
+ "?foo=?_weird?~`:[]stuff&bar=okay&&&&&&",
+ "?intlist=1",
+ "?intlist=1,2,3,4,5",
+ "?string=1a",
+ "?string=1,2,3,4,z",
+ "?string=1,2,3,4,[0]",
+ "?intlist=1,2,3,4,5&string=1,2,3,4,y"
+ };
+
+ const std::vector<const char *> invalidParams = {
+ "?/",
+ "?foo",
+ "?/foo",
+ "?foo/",
+ "?foo=",
+ "?foo=&",
+ "?=foo",
+ "? foo=bar",
+ "? &",
+ "?& ",
+ "?=&",
+ "?&=",
+ "? &&&",
+ "?& &&",
+ "?&& &",
+ "?=&&&",
+ "?&=&&",
+ "?&&=&"
+ };
+
+ const std::vector<const char *> validFragments = {
+ "",
+ "#",
+ "##",
+ "#?a=b",
+ "#fragment"
+ };
+
+ for (const auto &scheme : validSchemes) {
+ mgrUrl.setScheme(scheme);
+
+ for (const auto *magic : magicPrefixes) {
+
+ // all schemes except cache_object require magic path prefix bytes
+ if (scheme != AnyP::PROTO_CACHE_OBJECT && strlen(magic) <= 2)
+ continue;
+
+ /* Check the parser accepts all the valid cases */
+
+ for (const auto *action : validActions) {
+ for (const auto *param : validParams) {
+ for (const auto *frag : validFragments) {
+ try {
+ SBuf bits;
+ bits.append(magic);
+ bits.append(action);
+ bits.append(param);
+ bits.append(frag);
+ mgrUrl.path(bits);
+
+ (void)mgr->ParseUrl(mgrUrl);
+ } catch (...) {
+ std::cerr << std::endl
+ << "FAIL: " << mgrUrl
+ << Debug::Extra << "error: " << CurrentException << std::endl;
+ CPPUNIT_FAIL("rejected a valid URL");
+ }
+ }
+ }
+ }
+
+ /* Check that invalid parameters are rejected */
+
+ for (const auto *action : validActions) {
+ for (const auto *param : invalidParams) {
+ for (const auto *frag : validFragments) {
+ try {
+ SBuf bits;
+ bits.append(magic);
+ bits.append(action);
+ bits.append(param);
+ bits.append(frag);
+ mgrUrl.path(bits);
+
+ (void)mgr->ParseUrl(mgrUrl);
+
+ std::cerr << std::endl
+ << "FAIL: " << mgrUrl
+ << Debug::Extra << "error: should be rejected due to '" << param << "'" << std::endl;
+ } catch (const TextException &e) {
+ continue; // success. caught bad input
+ }
+ CPPUNIT_FAIL("failed to reject an invalid URL");
+ }
+ }
+ }
+ }
+ }
+}
diff --git a/src/tests/testCacheManager.h b/src/tests/testCacheManager.h
index 6d32d69e5..fee15846a 100644
--- a/src/tests/testCacheManager.h
+++ b/src/tests/testCacheManager.h
@@ -20,6 +20,7 @@ class testCacheManager : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST_SUITE( testCacheManager );
CPPUNIT_TEST( testCreate );
CPPUNIT_TEST( testRegister );
+ CPPUNIT_TEST( testParseUrl );
CPPUNIT_TEST_SUITE_END();
public:
@@ -28,6 +29,7 @@ public:
protected:
void testCreate();
void testRegister();
+ void testParseUrl();
};
#endif

23
backport-CVE-2021-28651.patch Normal file
View File

@ -0,0 +1,23 @@
From 47a085ff06598b64817875769022b8707a0af7db Mon Sep 17 00:00:00 2001
From: Amos Jeffries <yadij@users.noreply.github.com>
Date: Wed, 24 Feb 2021 00:53:21 +0000
Subject: [PATCH] Bug 5104: Memory leak in RFC 2169 response parsing (#778)
A temporary parsing buffer was not being released when
parsing completed.
---
src/urn.cc | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/urn.cc b/src/urn.cc
index 69c29b75f4e..72ab801a906 100644
--- a/src/urn.cc
+++ b/src/urn.cc
@@ -425,6 +425,7 @@ urnParseReply(const char *inbuf, const HttpRequestMethod& m)
}
debugs(52, 3, "urnParseReply: Found " << i << " URLs");
+ xfree(buf);
return list;
}

22
backport-CVE-2021-28662.patch Normal file
View File

@ -0,0 +1,22 @@
From 051824924c709bd6162a378f746fb859454c674e Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Tue, 16 Mar 2021 11:45:11 -0400
Subject: [PATCH] Merge pull request from GHSA-jjq6-mh2h-g39h
---
src/http/RegisteredHeaders.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/http/RegisteredHeaders.cc b/src/http/RegisteredHeaders.cc
index a4f96db2b78..84f177af2d8 100644
--- a/src/http/RegisteredHeaders.cc
+++ b/src/http/RegisteredHeaders.cc
@@ -37,7 +37,7 @@ HeaderTableRecord::HeaderTableRecord(const char *n, HdrType theId, HdrFieldType
const HeaderTableRecord&
HeaderLookupTable_t::lookup (const char *buf, const std::size_t len) const {
const HeaderTableRecord *r = HttpHeaderHashTable::lookup(buf, len);
- if (!r)
+ if (!r || r->id == Http::HdrType::OTHER)
return BadHdr;
return *r;
}

235
backport-CVE-2021-31806-CVE-2021-31808.patch Normal file
View File

@ -0,0 +1,235 @@
From 7024fb734a59409889e53df2257b3fc817809fb4 Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Wed, 31 Mar 2021 02:44:42 +0000
Subject: [PATCH] Handle more Range requests (#790)
Also removed some effectively unused code.
---
src/HttpHdrRange.cc | 17 -------------
src/HttpHeaderRange.h | 5 ++--
src/client_side.cc | 4 ++--
src/client_side_request.cc | 27 ++++++++++++++++++---
src/client_side_request.h | 7 +++++-
src/http/Stream.cc | 49 ++++++--------------------------------
6 files changed, 41 insertions(+), 68 deletions(-)
diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc
index e7179dc..5849144 100644
--- a/src/HttpHdrRange.cc
+++ b/src/HttpHdrRange.cc
@@ -526,23 +526,6 @@ HttpHdrRange::offsetLimitExceeded(const int64_t limit) const
return true;
}
-bool
-HttpHdrRange::contains(const HttpHdrRangeSpec& r) const
-{
- assert(r.length >= 0);
- HttpHdrRangeSpec::HttpRange rrange(r.offset, r.offset + r.length);
-
- for (const_iterator i = begin(); i != end(); ++i) {
- HttpHdrRangeSpec::HttpRange irange((*i)->offset, (*i)->offset + (*i)->length);
- HttpHdrRangeSpec::HttpRange intersection = rrange.intersection(irange);
-
- if (intersection.start == irange.start && intersection.size() == irange.size())
- return true;
- }
-
- return false;
-}
-
const HttpHdrRangeSpec *
HttpHdrRangeIter::currentSpec() const
{
diff --git a/src/HttpHeaderRange.h b/src/HttpHeaderRange.h
index 2103b64..352e749 100644
--- a/src/HttpHeaderRange.h
+++ b/src/HttpHeaderRange.h
@@ -78,7 +78,6 @@ public:
int64_t firstOffset() const;
int64_t lowestOffset(int64_t) const;
bool offsetLimitExceeded(const int64_t limit) const;
- bool contains(const HttpHdrRangeSpec& r) const;
std::vector<HttpHdrRangeSpec *> specs;
private:
@@ -100,9 +99,9 @@ public:
void updateSpec();
int64_t debt() const;
void debt(int64_t);
- int64_t debt_size; /* bytes left to send from the current spec */
+ int64_t debt_size = 0; /* bytes left to send from the current spec */
String boundary; /* boundary for multipart responses */
- bool valid;
+ bool valid = false;
};
#endif /* SQUID_HTTPHEADERRANGE_H */
diff --git a/src/client_side.cc b/src/client_side.cc
index 120c39c..9516166 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -724,8 +724,8 @@ clientPackRangeHdr(const HttpReply * rep, const HttpHdrRangeSpec * spec, String
* warning: assumes that HTTP headers for individual ranges at the
* time of the actuall assembly will be exactly the same as
* the headers when clientMRangeCLen() is called */
-int
-ClientHttpRequest::mRangeCLen()
+int64_t
+ClientHttpRequest::mRangeCLen() const
{
int64_t clen = 0;
MemBuf mb;
diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index a37f8d4..3c83e9e 100644
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1094,9 +1094,6 @@ clientInterpretRequestHeaders(ClientHttpRequest * http)
* iter up at this point.
*/
node->readBuffer.offset = request->range->lowestOffset(0);
- http->range_iter.pos = request->range->begin();
- http->range_iter.end = request->range->end();
- http->range_iter.valid = true;
}
}
@@ -1954,6 +1951,30 @@ ClientHttpRequest::setErrorUri(const char *aUri)
#include "client_side_request.cci"
#endif
+// XXX: This should not be a _request_ method. Move range_iter elsewhere.
+int64_t
+ClientHttpRequest::prepPartialResponseGeneration()
+{
+ assert(request);
+ assert(request->range);
+
+ range_iter.pos = request->range->begin();
+ range_iter.end = request->range->end();
+ range_iter.debt_size = 0;
+ const auto multipart = request->range->specs.size() > 1;
+ if (multipart)
+ range_iter.boundary = rangeBoundaryStr();
+ range_iter.valid = true; // TODO: Remove.
+ range_iter.updateSpec(); // TODO: Refactor to initialize rather than update.
+
+ assert(range_iter.pos != range_iter.end);
+ const auto &firstRange = *range_iter.pos;
+ assert(firstRange);
+ out.offset = firstRange->offset;
+
+ return multipart ? mRangeCLen() : firstRange->length;
+}
+
#if USE_ADAPTATION
/// Initiate an asynchronous adaptation transaction which will call us back.
void
diff --git a/src/client_side_request.h b/src/client_side_request.h
index 704802b..7ab3262 100644
--- a/src/client_side_request.h
+++ b/src/client_side_request.h
@@ -131,7 +131,7 @@ public:
dlink_node active;
dlink_list client_stream;
- int mRangeCLen();
+ int64_t mRangeCLen() const;
ClientRequestContext *calloutContext;
void doCallouts();
@@ -148,6 +148,11 @@ public:
/// neither the current request nor the parsed request URI are known
void setErrorUri(const char *errorUri);
+ /// Prepares to satisfy a Range request with a generated HTTP 206 response.
+ /// Initializes range_iter state to allow raw range_iter access.
+ /// \returns Content-Length value for the future response; never negative
+ int64_t prepPartialResponseGeneration();
+
/// Build an error reply. For use with the callouts.
void calloutsError(const err_type error, const int errDetail);
diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index 1370862..ff44496 100644
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -444,59 +444,27 @@ Http::Stream::buildRangeHeader(HttpReply *rep)
} else {
/* XXX: TODO: Review, this unconditional set may be wrong. */
rep->sline.set(rep->sline.version, Http::scPartialContent);
- // web server responded with a valid, but unexpected range.
- // will (try-to) forward as-is.
- //TODO: we should cope with multirange request/responses
- // TODO: review, since rep->content_range is always nil here.
- bool replyMatchRequest = contentRange != nullptr ?
- request->range->contains(contentRange->spec) :
- true;
+
+ // before range_iter accesses
+ const auto actual_clen = http->prepPartialResponseGeneration();
+
const int spec_count = http->request->range->specs.size();
- int64_t actual_clen = -1;
debugs(33, 3, "range spec count: " << spec_count <<
" virgin clen: " << rep->content_length);
assert(spec_count > 0);
/* append appropriate header(s) */
if (spec_count == 1) {
- if (!replyMatchRequest) {
- hdr->putContRange(contentRange);
- actual_clen = rep->content_length;
- //http->range_iter.pos = rep->content_range->spec.begin();
- (*http->range_iter.pos)->offset = contentRange->spec.offset;
- (*http->range_iter.pos)->length = contentRange->spec.length;
-
- } else {
- HttpHdrRange::iterator pos = http->request->range->begin();
- assert(*pos);
- /* append Content-Range */
-
- if (!contentRange) {
- /* No content range, so this was a full object we are
- * sending parts of.
- */
- httpHeaderAddContRange(hdr, **pos, rep->content_length);
- }
-
- /* set new Content-Length to the actual number of bytes
- * transmitted in the message-body */
- actual_clen = (*pos)->length;
- }
+ const auto singleSpec = *http->request->range->begin();
+ assert(singleSpec);
+ httpHeaderAddContRange(hdr, *singleSpec, rep->content_length);
} else {
/* multipart! */
- /* generate boundary string */
- http->range_iter.boundary = http->rangeBoundaryStr();
/* delete old Content-Type, add ours */
hdr->delById(Http::HdrType::CONTENT_TYPE);
httpHeaderPutStrf(hdr, Http::HdrType::CONTENT_TYPE,
"multipart/byteranges; boundary=\"" SQUIDSTRINGPH "\"",
SQUIDSTRINGPRINT(http->range_iter.boundary));
- /* Content-Length is not required in multipart responses
- * but it is always nice to have one */
- actual_clen = http->mRangeCLen();
-
- /* http->out needs to start where we want data at */
- http->out.offset = http->range_iter.currentSpec()->offset;
}
/* replace Content-Length header */
@@ -504,9 +472,6 @@ Http::Stream::buildRangeHeader(HttpReply *rep)
hdr->delById(Http::HdrType::CONTENT_LENGTH);
hdr->putInt64(Http::HdrType::CONTENT_LENGTH, actual_clen);
debugs(33, 3, "actual content length: " << actual_clen);
-
- /* And start the range iter off */
- http->range_iter.updateSpec();
}
}
--
2.23.0

111
backport-CVE-2021-33620.patch Normal file
View File

@ -0,0 +1,111 @@
From 6c9c44d0e9cf7b72bb233360c5308aa063af3d69 Mon Sep 17 00:00:00 2001
From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Fri, 2 Apr 2021 07:46:20 +0000
Subject: [PATCH] Handle more partial responses (#791)
---
src/HttpHdrContRange.cc | 14 ++++++++++++--
src/HttpHeaderRange.h | 7 +++++--
src/clients/Client.cc | 7 +++++--
src/http/Stream.cc | 9 +++++++--
4 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc
index 8270e0f11aa..79a507fb84c 100644
--- a/src/HttpHdrContRange.cc
+++ b/src/HttpHdrContRange.cc
@@ -161,9 +161,13 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str)
++p;
- if (*p == '*')
+ if (*p == '*') {
+ if (!known_spec(range->spec.offset)) {
+ debugs(68, 2, "invalid (*/*) content-range-spec near: '" << str << "'");
+ return 0;
+ }
range->elength = range_spec_unknown;
- else if (!httpHeaderParseOffset(p, &range->elength))
+ } else if (!httpHeaderParseOffset(p, &range->elength))
return 0;
else if (range->elength <= 0) {
/* Additional paranoidal check for BUG2155 - entity-length MUST be > 0 */
@@ -174,6 +178,12 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str)
return 0;
}
+ // reject unsatisfied-range and such; we only use well-defined ranges today
+ if (!known_spec(range->spec.offset) || !known_spec(range->spec.length)) {
+ debugs(68, 2, "unwanted content-range-spec near: '" << str << "'");
+ return 0;
+ }
+
debugs(68, 8, "parsed content-range field: " <<
(long int) range->spec.offset << "-" <<
(long int) range->spec.offset + range->spec.length - 1 << " / " <<
diff --git a/src/HttpHeaderRange.h b/src/HttpHeaderRange.h
index 6d93e72b2b8..bf54c8562ba 100644
--- a/src/HttpHeaderRange.h
+++ b/src/HttpHeaderRange.h
@@ -18,8 +18,11 @@
class HttpReply;
class Packable;
-/* http byte-range-spec */
-
+// TODO: Refactor to disambiguate and provide message-specific APIs.
+/// either byte-range-spec (in a request Range header)
+/// or suffix-byte-range-spec (in a request Range header)
+/// or byte-range part of byte-range-resp (in a response Content-Range header)
+/// or "*" part of unsatisfied-range (in a response Content-Range header)
class HttpHdrRangeSpec
{
MEMPROXY_CLASS(HttpHdrRangeSpec);
diff --git a/src/clients/Client.cc b/src/clients/Client.cc
index dfb13d053c3..2a2f0e8f878 100644
--- a/src/clients/Client.cc
+++ b/src/clients/Client.cc
@@ -520,8 +520,11 @@ Client::haveParsedReplyHeaders()
maybePurgeOthers();
// adaptation may overwrite old offset computed using the virgin response
- const bool partial = theFinalReply->contentRange();
- currentOffset = partial ? theFinalReply->contentRange()->spec.offset : 0;
+ currentOffset = 0;
+ if (const auto cr = theFinalReply->contentRange()) {
+ if (cr->spec.offset != HttpHdrRangeSpec::UnknownPosition)
+ currentOffset = cr->spec.offset;
+ }
}
/// whether to prevent caching of an otherwise cachable response
diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index 9e346b9d99d..d685a22306e 100644
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -171,12 +171,13 @@ Http::Stream::getNextRangeOffset() const
return start;
}
- } else if (reply && reply->contentRange()) {
+ } else if (const auto cr = reply ? reply->contentRange() : nullptr) {
/* request does not have ranges, but reply does */
/** \todo FIXME: should use range_iter_pos on reply, as soon as reply->content_range
* becomes HttpHdrRange rather than HttpHdrRangeSpec.
*/
- return http->out.offset + reply->contentRange()->spec.offset;
+ if (cr->spec.offset != HttpHdrRangeSpec::UnknownPosition)
+ return http->out.offset + cr->spec.offset;
}
return http->out.offset;
@@ -240,6 +241,10 @@ Http::Stream::socketState()
// did we get at least what we expected, based on range specs?
+ // this Content-Range does not tell us how many bytes to expect
+ if (bytesExpected == HttpHdrRangeSpec::UnknownPosition)
+ return STREAM_NONE;
+
if (bytesSent == bytesExpected) // got everything
return STREAM_COMPLETE;

14
squid.spec
View File

@ -2,7 +2,7 @@
Name: squid
Version: 4.9
Release: 7
Release: 8
Summary: The Squid proxy caching server
Epoch: 7
License: GPLv2+ and (LGPLv2+ and MIT and BSD and Public Domain)
@ -34,6 +34,12 @@ Patch13:CVE-2020-15810.patch
Patch14:CVE-2020-15811.patch
Patch15:CVE-2020-24606.patch
Patch16:backport-CVE-2020-25097.patch
Patch17:backport-CVE-2021-28651.patch
Patch18:backport-0001-CVE-2021-28652.patch
Patch19:backport-0002-CVE-2021-28652.patch
Patch20:backport-CVE-2021-28662.patch
Patch21:backport-CVE-2021-31806-CVE-2021-31808.patch
Patch22:backport-CVE-2021-33620.patch
Buildroot: %{_tmppath}/squid-4.9-1-root-%(%{__id_u} -n)
Requires: bash >= 2.0
@ -212,6 +218,12 @@ fi
chgrp squid /var/cache/samba/winbindd_privileged >/dev/null 2>&1 || :
%changelog
* Wed Jun 16 2021 xihaochen<xihaochen@huawei.com> - 4.9-8
- Type:cves
- ID:CVE-2021-28651 CVE-2021-28652 CVE-2021-28662 CVE-2021-31806 CVE-2021-31808 CVE-2021-33620
- SUG:NA
- DESC:fix CVE-2021-28651 CVE-2021-28652 CVE-2021-28662 CVE-2021-31806 CVE-2021-31808 CVE-2021-33620
* Wed Mar 31 2021 gaihuiying <gaihuiying1@huawei.com> - 4.9-7
- Type:cves
- ID:NA