From 021d0caa335637b493b9a1e45e2273c64659694f Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Mon, 19 Aug 2024 10:20:12 -0700 Subject: [PATCH] Perform NormalisePath & NormalisePathWin transformations in-place --- src/actions/transformations/normalise_path.cc | 60 +++++++------------ src/actions/transformations/normalise_path.h | 3 +- .../transformations/normalise_path_win.cc | 19 +----- 3 files changed, 24 insertions(+), 58 deletions(-) diff --git a/src/actions/transformations/normalise_path.cc b/src/actions/transformations/normalise_path.cc index c663b98a..dd2b64d2 100644 --- a/src/actions/transformations/normalise_path.cc +++ b/src/actions/transformations/normalise_path.cc @@ -25,23 +25,7 @@ NormalisePath::NormalisePath(const std::string &action) } bool NormalisePath::transform(std::string &value, const Transaction *trans) const { - int _changed = 0; - - char *tmp = reinterpret_cast( - malloc(sizeof(char) * value.size() + 1)); - memcpy(tmp, value.c_str(), value.size() + 1); - tmp[value.size()] = '\0'; - - int i = normalize_path_inplace((unsigned char *)tmp, - value.size(), 0, &_changed); - - std::string ret(""); - ret.assign(tmp, i); - free(tmp); - - const auto changed = ret != value; - value = ret; - return changed; + return normalize_path_inplace(value, false); } @@ -49,21 +33,22 @@ bool NormalisePath::transform(std::string &value, const Transaction *trans) cons * * IMP1 Assumes NUL-terminated */ -int NormalisePath::normalize_path_inplace(unsigned char *input, int input_len, - int win, int *changed) { +bool NormalisePath::normalize_path_inplace(std::string &val, const bool win) { unsigned char *src; unsigned char *dst; unsigned char *end; - int ldst = 0; int hitroot = 0; int done = 0; int relative; int trailing; - *changed = 0; + bool changed = false; /* Need at least one byte to normalize */ - if (input_len <= 0) return 0; + if(val.empty()) return false; + + auto input = reinterpret_cast(val.data()); + const auto input_len = val.length(); /* * ENH: Deal with UNC and drive letters? @@ -71,7 +56,6 @@ int NormalisePath::normalize_path_inplace(unsigned char *input, int input_len, src = dst = input; end = input + (input_len - 1); - ldst = 1; relative = ((*input == '/') || (win && (*input == '\\'))) ? 0 : 1; trailing = ((*end == '/') || (win && (*end == '\\'))) ? 1 : 0; @@ -82,11 +66,11 @@ int NormalisePath::normalize_path_inplace(unsigned char *input, int input_len, if (win) { if (*src == '\\') { *src = '/'; - *changed = 1; + changed = true; } if ((src < end) && (*(src + 1) == '\\')) { *(src + 1) = '/'; - *changed = 1; + changed = true; } } @@ -104,7 +88,7 @@ int NormalisePath::normalize_path_inplace(unsigned char *input, int input_len, /* Could it be an empty path segment? */ if ((src != end) && *src == '/') { /* Ignore */ - *changed = 1; + changed = true; goto copy; /* Copy will take care of this. */ } else if (*src == '.') { /* Could it be a back or self reference? */ @@ -141,25 +125,25 @@ int NormalisePath::normalize_path_inplace(unsigned char *input, int input_len, } } - if (done) goto length; /* Skip the copy. */ + if (done) goto skip_copy; /* Skip the copy. */ src++; - *changed = 1; + changed = true; } else if (dst == input) { /* Relative Self-reference? */ - *changed = 1; + changed = true; /* Ignore. */ - if (done) goto length; /* Skip the copy. */ + if (done) goto skip_copy; /* Skip the copy. */ src++; } else if (*(dst - 1) == '/') { /* Self-reference? */ - *changed = 1; + changed = true; /* Ignore. */ - if (done) goto length; /* Skip the copy. */ + if (done) goto skip_copy; /* Skip the copy. */ dst--; src++; } @@ -179,7 +163,7 @@ copy: && ((*(src + 1) == '/') || (win && (*(src + 1) == '\\'))) ) { src++; } - if (oldsrc != src) *changed = 1; + if (oldsrc != src) changed = true; /* Do not copy the forward slash to the root * if it is not a relative path. Instead @@ -187,27 +171,27 @@ copy: */ if (relative && (dst == input)) { src++; - goto length; /* Skip the copy */ + goto skip_copy; /* Skip the copy */ } } *(dst++) = *(src++); -length: - ldst = (dst - input); +skip_copy: + ; // nop for the goto label to work } /* Make sure that there is not a trailing slash in the * normalized form if there was not one in the original form. */ if (!trailing && (dst > input) && *(dst - 1) == '/') { - ldst--; dst--; } /* Always NUL terminate */ *dst = '\0'; - return ldst; + val.resize(dst - input); + return changed; } diff --git a/src/actions/transformations/normalise_path.h b/src/actions/transformations/normalise_path.h index d5b84085..248def7f 100644 --- a/src/actions/transformations/normalise_path.h +++ b/src/actions/transformations/normalise_path.h @@ -26,8 +26,7 @@ class NormalisePath : public Transformation { bool transform(std::string &value, const Transaction *trans) const override; - static int normalize_path_inplace(unsigned char *input, int input_len, - int win, int *changed); + static bool normalize_path_inplace(std::string &val, const bool win); }; } // namespace modsecurity::actions::transformations diff --git a/src/actions/transformations/normalise_path_win.cc b/src/actions/transformations/normalise_path_win.cc index 49f6989f..b60943e1 100644 --- a/src/actions/transformations/normalise_path_win.cc +++ b/src/actions/transformations/normalise_path_win.cc @@ -22,24 +22,7 @@ namespace modsecurity::actions::transformations { bool NormalisePathWin::transform(std::string &value, const Transaction *trans) const { - int _changed; - - char *tmp = reinterpret_cast( - malloc(sizeof(char) * value.size() + 1)); - memcpy(tmp, value.c_str(), value.size() + 1); - tmp[value.size()] = '\0'; - - int i = NormalisePath::normalize_path_inplace( - reinterpret_cast(tmp), - value.size(), 1, &_changed); - - std::string ret(""); - ret.assign(tmp, i); - free(tmp); - - const auto changed = ret != value; - value = ret; - return changed; + return NormalisePath::normalize_path_inplace(value, true); }