The IRC relay protocol's PASS handler compared the server password with
the client-supplied value using strcmp, leaking the password byte-by-byte
via response timing. This is the same class of bug fixed for the api and
weechat protocols, on a separate code path that did not go through
relay_auth_check_password_plain.
Extract the HMAC-then-constant-time-compare logic from
relay_auth_check_password_plain into relay_auth_password_equals, then
use it in both the plain-auth wrapper and the IRC PASS handler.
The relay authentication used non-constant-time comparisons (strcasecmp,
strcmp) to verify password hashes and plaintext passwords, allowing an
attacker to derive the expected hash byte-by-byte from response timing
and then authenticate without knowing the password.
- SHA/PBKDF2 hex hash comparisons: normalize the client-supplied hash to
uppercase and compare in constant time over the fixed expected length.
- Plaintext password comparison: HMAC-SHA256 both passwords with a fresh
per-call random key and compare the fixed-size MACs in constant time,
hiding both per-byte timing and the password length.
Add string_memcmp_constant_time helper in core, exposed via the plugin
API. Bump WEECHAT_PLUGIN_API_VERSION accordingly.
An authenticated relay client using the permessage-deflate websocket
extension could send a small compressed frame that decompresses to an
unbounded amount of data, exhausting all memory and crashing WeeChat.
The output buffer in relay_websocket_inflate is now capped to
WEBSOCKET_INFLATE_MAX_SIZE: frames decompressing beyond this limit are
rejected and the connection is closed.
Move the requirement checking and the dummy test where they are used.
Use REQUIRED instead of the manual FOUND check and error handling.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
With CMP0083 introduced with cmake 3.14, as we set the variable
CMAKE_POSITION_INDEPENDENT_CODE we can rely on the build system to do
the correct thing, across all the targets.
Since we require 3.18 (or 3.16 in the patched Debian/Ubuntu version),
which sets the policy to NEW we're all set.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Move the handling to the top-level, adding it _once_ to EXTRA_LIBS.
Thus avoiding some duplication across the board.
Note that final handling varies a bit, namely:
- OpenBSD/intl should be handled via the existing cmake/FindGettext.cmake
- Darwin/resolv should not be needed since commit e98a32373 ("core: check
if res_init requires linking with libresolv")
- the backtrace/execinfo handling has been consolidated and moved
In the unlikely case of unwanted over-linking, the platforms can add
`-Wl,--as-needed` to their linker flags. Something which is strongly
encouraged and has been the default across multiple (linux) distros for
years.
Alternatively, if move quirks are needed they should be handled in a
single place.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Move the handling to the top-level, adding it _once_ to EXTRA_LIBS.
Thus avoiding some duplication across the board.
This change technically adds an extra link for the unit tests, which
seemingly was omitted by mistake. Alternatively, the extra over-linking
won't be an issue in practise.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Move the handling to the top-level, adding it _once_ to EXTRA_LIBS.
Thus avoiding some duplication across the board.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Move the handling to the top-level, adding it _once_ to EXTRA_LIBS.
Thus avoiding some duplication across the board.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
In a handful of places we explicitly use add_dependencies() where the
exact same libraries are also (implicitly) added as dependencies via
target_link_libraries().
Remove the folder, which helps us remove some duplication with follow-up
patches.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
There is an issue with some IRC servers that may send a JOIN with self nick
once already on the channel, this results in a clear of the nicklist on the
second JOIN received.
This fix silently ignores the second self JOIN if the channel is already
joined (with at least one nick).