Skip to content

Commit

Permalink
Address reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
philwo committed Aug 18, 2023
1 parent 57fddf9 commit ecb449d
Showing 1 changed file with 19 additions and 32 deletions.
51 changes: 19 additions & 32 deletions src/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2206,59 +2206,46 @@ static string ShellEscape(const string& src) {
}
return result;
}
#endif

// Trim whitespace from the start of the provided string.
static inline void ltrim(std::string &s) {
s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](char ch) {
return std::isspace(ch) == 0;
}));
}

// Trim whitespace from the end of the provided string.
static inline void rtrim(std::string &s) {
s.erase(std::find_if(s.rbegin(), s.rend(), [](char ch) {
return std::isspace(ch) == 0;
}).base(), s.end());
}

// Trim whitespace from both ends of the provided string.
static inline void trim(std::string &s) {
rtrim(s);
ltrim(s);
const auto toRemove = [](char ch) { return std::isspace(ch) == 0; };
s.erase(s.begin(), std::find_if(s.begin(), s.end(), toRemove));
s.erase(std::find_if(s.rbegin(), s.rend(), toRemove).base(), s.end());
}
#endif

// use_logging controls whether the logging functions LOG/VLOG are used
// to log errors. It should be set to false when the caller holds the
// log_mutex.
static bool SendEmailInternal(const char*dest, const char *subject,
const char*body, bool use_logging) {
#ifndef GLOG_OS_EMSCRIPTEN
// We validate the provided email addresses using the same regular expression
// that HTML5 uses[1], except that we require the address to start with an
// alpha-numeric character. This is because we don't want to allow email
// addresses that start with a special character, such as a pipe or dash,
// which could be misunderstood as a command-line flag by certain versions
// of `mail` that are vulnerable to command injection.[2]
// [1] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
// [2] e.g. https://nvd.nist.gov/vuln/detail/CVE-2004-2771
const std::regex pattern("^[a-zA-Z0-9]"
"[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]*@[a-zA-Z0-9]"
"(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9]"
"(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$");

if (dest && *dest) {
// Split the comma-separated list of email addresses, validate each one and
// build a sanitized new comma-separated string without whitespace.
std::stringstream ss(dest);
std::istringstream ss(dest);
std::ostringstream sanitized_dests;
std::string s;
while (std::getline(ss, s, ',')) {
trim(s);
if (s.empty()) {
continue;
}
if (!std::regex_match(s, pattern)) {
// We validate the provided email addresses using the same regular
// expression that HTML5 uses[1], except that we require the address to
// start with an alpha-numeric character. This is because we don't want to
// allow email addresses that start with a special character, such as a
// pipe or dash, which could be misunderstood as a command-line flag by
// certain versions of `mail` that are vulnerable to command injection.[2]
// [1] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
// [2] e.g. https://nvd.nist.gov/vuln/detail/CVE-2004-2771
if (!std::regex_match(
s,
std::regex("^[a-zA-Z0-9]"
"[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]*@[a-zA-Z0-9]"
"(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9]"
"(?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$"))) {
if (use_logging) {
VLOG(1) << "Invalid destination email address:" << s;
} else {
Expand Down

0 comments on commit ecb449d

Please sign in to comment.