- Replaced pthread_mutex_t in modsecurity::operators::Pm with std::mutex
- Replaced pthread's thread usage in reading_logs_via_rule_message
example with std::thread.
- Simplified and modernized C++ code.
- Removed unnecessary includes of pthread.h
- As reported in #3054, the resolve methods in InMemoryPerProcess are
not acquiring a lock/mutex to prevent concurrent access to the data
structures that may be modified at the same time from other threads,
and thus triggering undefined behaviour.
- Replace inheritance of std::unordered_multimap data structure with
data member to prevent potential clients to use it without acquiring
the mutex to protect concurrent access.
- Replace pthreads lock with std C++11 std::shared_mutex
- Provides exclusive/shared lock access so that multiple readers can
access the data at the same time, but only one writer. this is used
to favor query performance by allowing more concurrent access to the
data until an update needs to be performed.
- Simplifies acquisition and disposal of lock/mutex with
std::lock_guard, which has RAII semantics.
- NOTE: Because std::shared_mutex is not recursive, calls to another
function that tries to acquire the lock will fail. Introduced
__store & __updateFirst helper methods to workaround this.
- Updates to InMemoryPerProcess::resolveFirst
- Updated the code to store the expired var in 'expiredVars' to delete
them after iterating over the range (and releasing the read lock, as
'delIfExpired' needs to acquire it for exclusive access), as the
current call to 'delIfExpired' would invalidate the range triggering
undefined behaviour on the following iteration.
- Noticed that in commit 118e1b3 the call to 'delIfExpired' in this
function is done using 'it->second.getValue()'' instead of
'it->first', which seems incorrect (based on similar code in other
resolveXXX functions).
- Updated InMemoryPerProcess::delIfExpired to use 'std::find_if' (with a
lambda that matches both the key and the 'isExpired' condition)
because the data structure is a multimap. The version introduced in
commit 118e1b3 could find an entry (not necessarily the first, because
the map is unordered) where 'isExpired' is 'false' and exit, while
another entry could be expired.
This issue was initially reported by @michaelgranzow-avi on #2296.
@airween made an initial attempt to provide a fixed at #2107; As a
consequence of the pull request review - provided by @victorhora,
@zimmerle, and @michaelgranzow-avi - @airween made a second attempt
at #2297. After reviewing by @martinhsv, @zimmerle, I have absorbed
the essential pieces from @airween patch into this one.
This patch differs from @airween's because @airween's patches were
partially working: Key exclusions with regex weren't covered, same
for anchored variables (e.g. ARGS). During the review, I have
highlighted the importance of having elementary test cases. A simple
test case on ARGS could spot the issue. Since that is an important
fix, I don't want to hold this for one more review cycle; therefore,
I am committing the fix myself.
Thank you all involved in the solution of this very own issue.
This commit fixes quite a few odd things in regex code:
* Lack of encapsulation.
* Non-method functions for matching without retrieving all groups.
* Regex class being copyable without proper copy-constructor (potential UAF
and double free due to pointer members m_pc and m_pce).
* Redundant SMatch::m_length, which always equals to match.size() anyway.
* Weird SMatch::size_ member which is initialized only by one of the three matching
functions, and equals to the return value of that function anyways.
* Several places in code having std::string value instead of reference.
