Forum teuk.org

Security and Correctness Pass (May 2026)

in Mediabot · started by TeuK · 1w ago

TeuK · 1w ago

Mediabot v3 — Security and Correctness Pass (May 2026)

Context

Audit of snapshot 20260503. Five modules touched.

The focus of this pass is authentication security and correctness:

dead database statement handle
missing brute-force guard
wildcard matching bug in the ban engine
Partyline quality-of-life improvements
Prometheus metrics correctness

This is a security and reliability pass, not a cosmetic cleanup.


Bug fixes

B1 — LoginCommands::checkAuth leaked a statement handle on bad password

Before:

if (my $ref = $sth->fetchrow_hashref()) {
    ...
    return 1;
} else {
    return 0;   # $sth->finish never reached
}
$sth->finish;   # dead code

The $sth->finish call was placed after the else block, but the else branch returned before reaching it.

That meant every failed password attempt left the statement handle unfinished.

On a busy server with repeated login attempts, this could eventually contribute to server-side handle/cursor exhaustion and database instability.

checkAuth was refactored so the statement handle is finished immediately after fetchrow_hashref, before any return path.


B2 — userLogin_ctx had no dedicated brute-force protection

The login command accepted repeated password attempts with no authentication-specific throttle.

There was already a generic checkNickFlood guard, but that applies to all commands uniformly and is not strict enough for a login endpoint.

A dedicated _login_failures counter is now maintained per nick.

Behavior:

5 failed attempts
60-second window
further attempts are refused until the window expires
counter resets on successful login
counter expires naturally when the window rolls over

This gives the login command its own brute-force guard without affecting unrelated commands.


B3 — ChannelBan::active_ban_for_mask used SQL equality instead of IRC wildcard matching

Before:

WHERE id_channel = ? AND mask = ? AND active = 1

That is wrong for IRC ban masks.

Ban masks are glob-style patterns, for example:

*!*evil@*.wanadoo.fr
*!~baduser@*
badnick!*@*

SQL equality only matches the exact same string. A stored ban such as:

*!*evil@*.wanadoo.fr

would not match:

badnick!~evil@host.wanadoo.fr

even though the IRC ban mask clearly covers it.

The query now fetches active bans for the channel and applies IRC wildcard matching in Perl through a new helper:

_mask_matches_irc()

Matching rules:

*  -> .*
?  -> .
case-insensitive comparison

The most recently created matching ban is returned.

This makes the persistent ban engine behave like real IRC ban masks.


B4 — make_password_hash was called without an eval safety net

checkAuth called:

make_password_hash($sPassword)

directly.

If the function is not properly imported after a refactor, or if the hashing path throws unexpectedly, the exception can propagate through the Net::Async::IRC event loop.

That is bad for a long-running bot.

The call is now wrapped in:

eval { ... }

with a log entry on failure.

The authentication code now fails safely instead of risking an uncaught exception.


B5 — .ban did not invalidate the .stat cache

.unban already invalidated the Partyline stat cache with:

delete $self->{_stat_cache};

.ban did not.

That meant .stat could continue showing the previous state for up to the cache lifetime after a new ban was added.

The cache is now invalidated after .ban too.


Improvements

A2 — Login failure counter integrated with brute-force guard

The login failure tracking now serves two purposes:

enforce the brute-force window
provide useful DEBUG3 logging

Unknown-user and bad-password events increment the per-nick counter.

The counter is cleared on successful login and resets automatically when the 60-second window expires.


A3 — active_ban_for_mask now supports real IRC wildcards

This is both a bug fix and a functional improvement.

Persistent bans such as:

*!*@*.isp.fr
*!~evil@*

now actually enforce against matching users.

This makes the channel ban system much closer to how IRC operators expect bans to behave.


A6 — mbRehash_ctx re-attaches the logger to Conf after reload

rehash_runtime_state() reloads the config file, but the Conf object’s internal logger reference was not refreshed.

After a rehash, if MEDIABOT_DEBUG_CONF=1 was set, missing-key warnings could fall back to STDERR because set_logger() had not been called again.

mbRehash_ctx now calls:

$self->{conf}->set_logger($self->{logger})

after a successful rehash.

This keeps configuration diagnostics routed through the normal Mediabot logger.


A7 — .whois <nick> from the Partyline

A new Partyline command was added:

.whois <nick>

Example:

.whois teuk
WHOIS sent for teuk...
[311] teuk ~user host.example.com * :Real name
[319] teuk :#boulets #testchan
[318] End of WHOIS

The command sends a WHOIS request to the IRC server and routes the reply lines back to the requesting Partyline session.

The requesting session fd is stored in:

$bot->{_partyline_whois_fd}

and read by the WHOIS reply handlers.

The command is also listed in .help and increments the Partyline metrics counter.


A8 — mediabot_auth_sessions_total declared with HELP and TYPE

The gauge:

mediabot_auth_sessions_total

was already being updated in Auth.pm, but it was not declared in Metrics.pm.

Prometheus could still scrape the metric, but the exposition was missing the expected metadata lines:

# HELP ...
# TYPE ...

The gauge is now declared at startup alongside the other auth metrics:

# HELP mediabot_auth_sessions_total Current in-memory authenticated sessions
# TYPE mediabot_auth_sessions_total gauge
mediabot_auth_sessions_total 2

This makes the metrics output cleaner and more Prometheus-friendly.


Files changed

File Items
Mediabot/LoginCommands.pm B1, B2, B4, A2
Mediabot/ChannelBan.pm B3, A3
Mediabot/Partyline.pm B5, A7
Mediabot/AdminCommands.pm A6
Mediabot/Metrics.pm A8

Suggested commit

🛡️ Protego Revelio: harden login, bans, Partyline WHOIS and auth metrics

Summary

This pass improves several sensitive areas of Mediabot v3:

authentication is safer
failed logins are throttled
database handles are cleaned up correctly
persistent bans now match IRC masks properly
Partyline .ban and .stat behave more consistently
Partyline gains a useful .whois command
auth session metrics are properly declared
rehash keeps config logging attached

The result is a bot that behaves more like a serious long-running IRC service: safer authentication, better operator tools, more correct ban enforcement, and cleaner observability.

You must be logged in to reply.