From 547260acd15fbcd8dfae1fee11905c7808c001a6 Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Wed, 15 Jun 2016 16:10:32 +0000 Subject: [PATCH 01/10] Add missing 'retrun's for functions declared return value. This change fixes SIGILLs on executable built with clang 3.4. Tested against FreeBSD 10.3. --- src/actions/ctl_audit_log_parts.cc | 2 ++ src/parser/driver.cc | 1 + 2 files changed, 3 insertions(+) diff --git a/src/actions/ctl_audit_log_parts.cc b/src/actions/ctl_audit_log_parts.cc index b9127dfe2d..136bb05e61 100644 --- a/src/actions/ctl_audit_log_parts.cc +++ b/src/actions/ctl_audit_log_parts.cc @@ -31,6 +31,8 @@ bool CtlAuditLogParts::init(std::string *error) { } else { mPartsAction = 1; } + + return true; } bool CtlAuditLogParts::evaluate(Rule *rule, Transaction *transaction) { diff --git a/src/parser/driver.cc b/src/parser/driver.cc index 34f946a9da..4a6bb27321 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -46,6 +46,7 @@ int Driver::addSecMarker(std::string marker) { rule->phase = i; rules[i].push_back(rule); } + return 0; } From 1b01ffe0b55b8527da91e3e80783b6bb0c10e36b Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Wed, 15 Jun 2016 16:18:46 +0000 Subject: [PATCH 02/10] Use explicit variable size for copying char. For some reason plain call to "ret.append(&b)" copy 32 bit of data. This change unbreaks CmdLine unit tests for FreeBSD 10, CentOS 7, RHEL 7 and Debian 8. --- src/actions/transformations/cmd_line.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/transformations/cmd_line.cc b/src/actions/transformations/cmd_line.cc index 7d567fcfd6..96b79876be 100644 --- a/src/actions/transformations/cmd_line.cc +++ b/src/actions/transformations/cmd_line.cc @@ -72,7 +72,7 @@ std::string CmdLine::evaluate(std::string value, /* copy normal characters */ default : char b = std::tolower(a); - ret.append(&b); + ret.append(&b, 1); space = 0; break; } From 3f2adc89e67c02f6e24a68ff6258d3bfc482e5fe Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Wed, 15 Jun 2016 16:32:10 +0000 Subject: [PATCH 03/10] Enforce bison requirement to 3.0.4. Previous versions of bison proven to generate broken code which caused to assert() regression tests of libmodsecurity for clang 3.4 and gcc 4.8. Upgrading bison to 3.0.4 solved mentioned issues for FreeBSD 10, CentOS 7, RHEL 7 and Ubuntu 14. --- src/parser/seclang-parser.yy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index ea977c06fa..4627d9013b 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -1,5 +1,5 @@ %skeleton "lalr1.cc" /* -*- C++ -*- */ -%require "3.0" +%require "3.0.4" %defines %define parser_class_name {seclang_parser} %define api.token.constructor From e4f7bb3102e3c4a85bd95d4551ffc003c0fd5969 Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Wed, 15 Jun 2016 16:54:47 +0000 Subject: [PATCH 04/10] Use PCRE for regular expressions instead of C++ regex library. C++ regex library proven to be unusable for gcc 4.8 and earlier version, so reimplement code using PCRE library in order to build workable version of unit_test executable for CentOS 7, RHEL 7, Ubuntu 14 and SUSE Linux 12. --- test/unit/unit_test.cc | 70 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/test/unit/unit_test.cc b/test/unit/unit_test.cc index 9cdcaf82be..2c6f00a772 100644 --- a/test/unit/unit_test.cc +++ b/test/unit/unit_test.cc @@ -13,15 +13,29 @@ * */ +/* + * Enforce using PCRE for regular expressions instead of C++ regex library. + * + * Since regex library proven to be unusable for gcc 4.8 and earlier versions, + * this define is required to build workable version of executable for CentOS 7, + * RHRL 7, Ubuntu 14.04 and SUSE Linux 12. + */ +#define PCRE_ONLY 1 + #include "unit/unit_test.h" #include +#ifdef PCRE_ONLY +#include +#endif #include #include #include #include +#ifndef PCRE_ONLY #include +#endif #include #include "common/colors.h" @@ -44,7 +58,19 @@ std::string string_to_hex(const std::string& input) { return output; } - +#ifdef PCRE_ONLY +void replaceAll(std::string *s, char *search, + const char replace) { + for (size_t pos = 0; ; pos += 0) { + pos = s->find(search, pos); + if (pos == std::string::npos) { + break; + } + s->erase(pos, strlen(search)); + s->insert(pos, &replace, 1); + } +} +#else void replaceAll(std::string *s, const std::string &search, const char replace) { for (size_t pos = 0; ; pos += 0) { @@ -56,9 +82,49 @@ void replaceAll(std::string *s, const std::string &search, s->insert(pos, &replace, 1); } } +#endif void json2bin(std::string *str) { +#ifdef PCRE_ONLY + pcre *re; + const char *emsg; + int ov[5], eoffs, rc; + char u[7], *cptr; + + re = pcre_compile("\\\\x([a-z0-9A-Z]{2})", 0, &emsg, &eoffs, NULL); + if (!re) + return; + + while ((rc = pcre_exec(re, NULL, str->c_str(), str->length(), 0, 0, ov, 5)) >= 0) { + unsigned int p; + + memset(u, 0, sizeof(u)); + cptr = (char *) str->c_str(); cptr += ov[0]; + for (p = 0; p < (ov[1]-ov[0]); p++) { u[p] = *(cptr+p); } + + sscanf((char *)(u+2), "%x", &p); + replaceAll(str, (char *)u, p); + } + pcre_free(re); + + re = pcre_compile("\\\\u([a-z0-9A-Z]{4})", 0, &emsg, &eoffs, NULL); + if (!re) + return; + + while ((rc = pcre_exec(re, NULL, str->c_str(), str->length(), 0, 0, ov, 5)) >= 0) { + unsigned int p; + + memset(u, 0, sizeof(u)); + cptr = (char *) str->c_str(); cptr += ov[0]; + for (p = 0; p < (ov[1]-ov[0]); p++) { u[p] = *(cptr+p); } + + sscanf((char *)(u+2), "%4x", &p); + replaceAll(str, (char *)u, p); + } + pcre_free(re); + +#else std::regex re("\\\\x([a-z0-9A-Z]{2})"); std::regex re2("\\\\u([a-z0-9A-Z]{4})"); std::smatch match; @@ -70,6 +136,7 @@ void json2bin(std::string *str) { sscanf(toBeReplaced.c_str(), "%x", &p); replaceAll(str, match.str(), p); } + while (std::regex_search(*str, match, re2) && match.size() > 1) { unsigned int p; std::string toBeReplaced = match.str(); @@ -77,6 +144,7 @@ void json2bin(std::string *str) { sscanf(toBeReplaced.c_str(), "%4x", &p); replaceAll(str, match.str(), p); } +#endif /* replaceAll(str, "\\0", '\0'); From 92a90d3caf08a5ba31066827008d1b912afe662a Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Thu, 16 Jun 2016 13:43:56 +0000 Subject: [PATCH 05/10] Revert previous change in favour of internal Regex implementation. --- test/unit/unit_test.cc | 70 +----------------------------------------- 1 file changed, 1 insertion(+), 69 deletions(-) diff --git a/test/unit/unit_test.cc b/test/unit/unit_test.cc index 2c6f00a772..9cdcaf82be 100644 --- a/test/unit/unit_test.cc +++ b/test/unit/unit_test.cc @@ -13,29 +13,15 @@ * */ -/* - * Enforce using PCRE for regular expressions instead of C++ regex library. - * - * Since regex library proven to be unusable for gcc 4.8 and earlier versions, - * this define is required to build workable version of executable for CentOS 7, - * RHRL 7, Ubuntu 14.04 and SUSE Linux 12. - */ -#define PCRE_ONLY 1 - #include "unit/unit_test.h" #include -#ifdef PCRE_ONLY -#include -#endif #include #include #include #include -#ifndef PCRE_ONLY #include -#endif #include #include "common/colors.h" @@ -58,19 +44,7 @@ std::string string_to_hex(const std::string& input) { return output; } -#ifdef PCRE_ONLY -void replaceAll(std::string *s, char *search, - const char replace) { - for (size_t pos = 0; ; pos += 0) { - pos = s->find(search, pos); - if (pos == std::string::npos) { - break; - } - s->erase(pos, strlen(search)); - s->insert(pos, &replace, 1); - } -} -#else + void replaceAll(std::string *s, const std::string &search, const char replace) { for (size_t pos = 0; ; pos += 0) { @@ -82,49 +56,9 @@ void replaceAll(std::string *s, const std::string &search, s->insert(pos, &replace, 1); } } -#endif void json2bin(std::string *str) { -#ifdef PCRE_ONLY - pcre *re; - const char *emsg; - int ov[5], eoffs, rc; - char u[7], *cptr; - - re = pcre_compile("\\\\x([a-z0-9A-Z]{2})", 0, &emsg, &eoffs, NULL); - if (!re) - return; - - while ((rc = pcre_exec(re, NULL, str->c_str(), str->length(), 0, 0, ov, 5)) >= 0) { - unsigned int p; - - memset(u, 0, sizeof(u)); - cptr = (char *) str->c_str(); cptr += ov[0]; - for (p = 0; p < (ov[1]-ov[0]); p++) { u[p] = *(cptr+p); } - - sscanf((char *)(u+2), "%x", &p); - replaceAll(str, (char *)u, p); - } - pcre_free(re); - - re = pcre_compile("\\\\u([a-z0-9A-Z]{4})", 0, &emsg, &eoffs, NULL); - if (!re) - return; - - while ((rc = pcre_exec(re, NULL, str->c_str(), str->length(), 0, 0, ov, 5)) >= 0) { - unsigned int p; - - memset(u, 0, sizeof(u)); - cptr = (char *) str->c_str(); cptr += ov[0]; - for (p = 0; p < (ov[1]-ov[0]); p++) { u[p] = *(cptr+p); } - - sscanf((char *)(u+2), "%4x", &p); - replaceAll(str, (char *)u, p); - } - pcre_free(re); - -#else std::regex re("\\\\x([a-z0-9A-Z]{2})"); std::regex re2("\\\\u([a-z0-9A-Z]{4})"); std::smatch match; @@ -136,7 +70,6 @@ void json2bin(std::string *str) { sscanf(toBeReplaced.c_str(), "%x", &p); replaceAll(str, match.str(), p); } - while (std::regex_search(*str, match, re2) && match.size() > 1) { unsigned int p; std::string toBeReplaced = match.str(); @@ -144,7 +77,6 @@ void json2bin(std::string *str) { sscanf(toBeReplaced.c_str(), "%4x", &p); replaceAll(str, match.str(), p); } -#endif /* replaceAll(str, "\\0", '\0'); From d83987c2fc4ca5b2dab4e3ac0d3014704116d336 Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Thu, 16 Jun 2016 13:49:17 +0000 Subject: [PATCH 06/10] Use internal PCRE based implementation of regular expressions instead of std C++ regex library. C++ regex library proven to be unusable for gcc 4.8 and earlier version, so reimplement code using PCRE library in order to build workable version of unit_test executable for CentOS 7, RHEL 7, Ubuntu 14 and SUSE Linux 12. --- src/utils/regex.h | 1 + test/unit/unit_test.cc | 14 +++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/utils/regex.h b/src/utils/regex.h index c964d4e7e2..0ee974b5bc 100644 --- a/src/utils/regex.h +++ b/src/utils/regex.h @@ -42,6 +42,7 @@ class SMatch { public: SMatch() : size_(0) { } size_t size() { return size_; } + std::string str() { return match; } int size_; std::string match; }; diff --git a/test/unit/unit_test.cc b/test/unit/unit_test.cc index 9cdcaf82be..5bb7d85baf 100644 --- a/test/unit/unit_test.cc +++ b/test/unit/unit_test.cc @@ -21,11 +21,11 @@ #include #include #include -#include #include #include "common/colors.h" #include "src/utils.h" +#include "src/utils/regex.h" namespace modsecurity_test { @@ -44,7 +44,6 @@ std::string string_to_hex(const std::string& input) { return output; } - void replaceAll(std::string *s, const std::string &search, const char replace) { for (size_t pos = 0; ; pos += 0) { @@ -59,18 +58,19 @@ void replaceAll(std::string *s, const std::string &search, void json2bin(std::string *str) { - std::regex re("\\\\x([a-z0-9A-Z]{2})"); - std::regex re2("\\\\u([a-z0-9A-Z]{4})"); - std::smatch match; + modsecurity::Utils::Regex re("\\\\x([a-z0-9A-Z]{2})"); + modsecurity::Utils::Regex re2("\\\\u([a-z0-9A-Z]{4})"); + modsecurity::Utils::SMatch match; - while (std::regex_search(*str, match, re) && match.size() > 1) { + while (modsecurity::Utils::regex_search(*str, &match, re) && match.size() > 0) { unsigned int p; std::string toBeReplaced = match.str(); toBeReplaced.erase(0, 2); sscanf(toBeReplaced.c_str(), "%x", &p); replaceAll(str, match.str(), p); } - while (std::regex_search(*str, match, re2) && match.size() > 1) { + + while (modsecurity::Utils::regex_search(*str, &match, re2) && match.size() > 0) { unsigned int p; std::string toBeReplaced = match.str(); toBeReplaced.erase(0, 2); From aa88fd67b62fb81070a969788821f473ceb0397d Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Thu, 16 Jun 2016 15:57:36 +0000 Subject: [PATCH 07/10] Fix compilation for libpthread users. --- examples/multithread_c/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/multithread_c/Makefile.am b/examples/multithread_c/Makefile.am index b89c840554..e47e3b21ea 100644 --- a/examples/multithread_c/Makefile.am +++ b/examples/multithread_c/Makefile.am @@ -8,6 +8,7 @@ multi_SOURCES = \ multi_LDADD = \ -L$(top_builddir)/src/.libs/ \ -lmodsecurity \ + -lpthread \ $(YAJL_LDFLAGS) \ $(GEOIP_LDFLAGS) \ $(GLOBAL_LDADD) From c9293993270bd177aed69e8001bd2964d93481fc Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Thu, 30 Jun 2016 12:06:41 +0000 Subject: [PATCH 08/10] Unbreak build with custom location of libyajl.so --- src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index befb241255..868ac9429b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -253,7 +253,7 @@ libmodsecurity_la_LIBADD = \ $(GEOIP_LDFLAGS) $(GEOIP_LDADD) \ @LEXLIB@ \ $(PCRE_LDADD) \ - $(YAJL_LDADD) \ + $(YAJL_LDFLAGS) $(YAJL_LDADD) \ $(LIBXML2_LDADD) \ ../others/libinjection.la \ libmbedtls.la From 98a12ed0606f2e0fb7cfac09a59f5a5920da3ecc Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Mon, 4 Jul 2016 13:26:32 +0000 Subject: [PATCH 09/10] Add explicit 'return true;' for Transaction::extractArguments() Unbreaks runtime for FreeBSD 10 (clang generated code) --- src/transaction.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/transaction.cc b/src/transaction.cc index 1f6a6f5d46..8609af5579 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -290,6 +290,8 @@ bool Transaction::extractArguments(const std::string &orig, free(key_c); free(value_c); } + + return true; } @@ -1740,7 +1742,7 @@ extern "C" int msc_request_body_from_file(Transaction *transaction, /** * @name msc_process_response_headers - * @brief Perform the analysis on the response readers. + * @brief Perform the analysis on the response headers. * * This function perform the analysis on the response headers, notice however * that the headers should be added prior to the execution of this function. From 251d1d47fbbd963d88da8dd6514d9fbcea634dd2 Mon Sep 17 00:00:00 2001 From: Alexey Zelkin Date: Mon, 4 Jul 2016 13:27:32 +0000 Subject: [PATCH 10/10] Fix typo (= vs ==) --- src/actions/allow.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/actions/allow.h b/src/actions/allow.h index 926c10c259..e8ac4ac3e3 100644 --- a/src/actions/allow.h +++ b/src/actions/allow.h @@ -64,11 +64,11 @@ class Allow : public Action { static std::string allowTypeToName (AllowType a) { if (a == NoneAllowType) { return "None"; - } else if (a = RequestAllowType) { + } else if (a == RequestAllowType) { return "Request"; - } else if (a = PhaseAllowType) { + } else if (a == PhaseAllowType) { return "Phase"; - } else if (a = FromNowOneAllowType) { + } else if (a == FromNowOneAllowType) { return "FromNowOne"; } else { return "Unknown";