This is mainly due to licensing. The libmaxminddb library uses the
Apache license, which meant if we would compile it in by default it
would effectively transform our "GPLv2 or later" to "GPLv3 or later".
Our implementation is ISC licensed, so we can include and enable it
by default and keep things at "GPLv2 or later". This is also why we
used geoip_classic in the first place as default and compiled in,
and not the mmdb variant.
The mmdb.c is based on the specification, using the Go implementation
as a reference during development (ISC licensed), initially implemented
with the help of Claude Opus 4.6. After that substantial changes were
made to make it match UnrealIRCd's style and to make things less error
prone: C style changes, allocation and zero termination of strings in
the library, auto-NULL in variadic functions so the caller cannot
forget NULL there (similar to our unreal_log/do_unreal_log), using
enums as the return type instead of int (similar to curl), adding
doxygen docs, etc.
This also means the old mmdb library dependency has been dropped,
including from configure/autoconf.
At the moment we still use the geoip classic library by default,
including those DB files. The idea is we will switch over sometime
later after this current new MMDB stuff has received more testing.
This also makes us more flexible, since .mmdb files have become the
de-facto standard for pretty much all geoip vendors.
This module is rarely used but analysis showed that there was an
OOB write in the country name, and two small off-by-ones in code
and continent.
Again, this only matters if the CSV file you are importing is bad
or malicious. And we use stack protection in UnrealIRCd so this
should then "only" cause a crash.
For the extbans that we ship, no problem, as this isn't used in
any of our extbans, but for third party it may matter, or for us
in the future.
Just something we came across while looking into the issue from
previous commit.
This affects servers without NEXTBANS, such as anope 2.0.x series
(anope 2.1.x is not affected as it supports NEXTBANS).
Non-NEXTBANS servers only support letter extbans so we are supposed
to convert ~security-group:known-users to ~G:known-users when sending
to such a server, in unreal_server_compat. And we did this well for
the MODE command for +beI. In SJOIN we did this correctly for +b/+e
but not for +I due to a silly code mistake.
This bug is present since 6.0.0 but wasn't noticed until now.
To be a real problem you need something like:
1. Anope 2.0.x series (or other services without NEXTBANS)
2. A channel with +I extbans
3. KEEPMODES set on that channel
Then what happens is when services boot:
1. UnrealIRCd will sync with anope 2.0.x and incorrectly send
named bans, which will confuse anope. But nothing strange
happens yet at this point.
2. Then on next server sync (eg anope restart or unreal restart)
anope will try to restore these but they end up with weird
entries like +I *!*@~security-group:known-users
(note the *!*@ prefix)
And it should be noted that this would also happen in a situation
with UnrealIRCd 5 + UnrealIRCd 6 servers, but UnrealIRCd 5 is
End Of Life anyway.
Reported by BlackBishop and Sadie two days ago. Thanks!
NOTE: Linked servers are considered trusted in UnrealIRCd.
This is not exploitable beyond a crash, due to -fstack-protector-all,
a hardening compiler flag we added many years ago. Even without
that flag it would be rather difficult, and i didn't manage to,
but this should never happen anyway since this flag is only
missing in gcc/clang versions that are more than 15 years old.
This issue was introduced by the move to CMD_BIGLINES in
6c5de62c18 in 6.2.2 release.
In particular, this disables default +F for #__SYNC__ channels.
The test suite has a "+F off" but when on 3 servers, each 75
clones are connecting, the MODE is too late and the join limit
is already reached sometimes. Causing tests to fail.
because it has no internet access, like when fetching the repository
(modules.list file) of 3rd party modules.
Previously I had..
url_start_async(request);
synchronous_http_request_in_progress = 1;
.. which worked fine for the "cannot connect case", like port blocked
or timeout connecting. But if DNS fails then the step of setting
synchronous_http_request_in_progress = -1 (so failed) already happens
during the url_start_async(request); call, and then the line after it
sets 'synchronous_http_request_in_progress = 1;' so we miss that it
failed and wait in the I/O loop forever.
Simply swapping the two lines of code fixes this.
The other change is that when running the ModuleManager in "make" we should
ignore the exit code. I probably broke that while refactoring and adding
non-zero exit codes in de modulemanager past few months for this release.
in NameList, Tag, Watch and HistoryLogLine.
This does mean the allocation routines need a +1 everywhere, but
I think I got all of them. I also don't see them being used directly
in such a way in 3rd party modules (which is logical, as they
should use the API and not allocate such structs directly).
Also, SpamExcept has been removed as it was not used anywhere.
the particular extended ban module if you don't want it.
For example, if you include the default modules.default.conf and, say,
you don't want ~quiet extbans then you add this in your unrealircd.conf:
blacklist-module "extbans/quiet";
This would cause a bit of a mess, that usually would be resolved a few
seconds later, but still a mess. I had this on irc*.unrealircd.org
myself when rerouting a server from a backup-hub to primary-hub
a few months ago.
This is not an issue now in all code paths, but if someone accidentally uses
SupportXYZ() without checking IsServer() then it would be an issue.
In the past we used client->local->proto for client flags as well, but this
has been split off to client->local->caps a while ago.
I guess we should rename client->local->proto to something more server-ish
in a later major release to indicate this as well.
and callback data in non-DEBUGMODE. Also because exposing pointers like
this can defeat ASLR. These STATS are oper-only though, but hey, defense in
depth... and the pointer values don't make sense to non-devs anyway,
so why show them in the first place.
We use mtag_add_issued_by() to prepare it but then pass NULL
in do_cmd() so it was basically useless.
Also compile fix for previous (forgot to git ammend)
Previously it showed this warning and said "Allowing user .. in unchecked"
when the user got shunend by CBL. Usually harmless but.. had a report
where it possibly was not (though that was an older UnrealIRCd version).
In any case, confusing, solved now!
Reject it with an ERR_INVALIDMODEPARAM, just like we do for +k.
I think the higher number transforming is fine, but this <=0 transformation
is odd as it almost never is what the user actually intended.
In S2S traffic we still transform, as rejecting there is more problematic,
(causing a desync) and transforming it there is not a major issue, anyway.
Reported by ProgVal in https://bugs.unrealircd.org/view.php?id=6602
since the message/notice would not make it through either.
This also means someone can no longer iterate through users to see who
is +D/+R by sending a "silent" TAGMSG. (Silent in the sense that the
end-user usually would not have noticed)
Suggested in https://bugs.unrealircd.org/view.php?id=6579 by zw32h (I think)
This also means HOOKTYPE_CAN_SEND_TO_USER now allows you to NOT to
set errmsg, to silently drop a message. Previously we would crash
deliberately on such a situation to enforce that all modules would
set a proper errmsg.
(commit 0cf0c0faa2)
This was caused by register_user() being called twice, while it should
only have been called if !IsUser().
Reported by ProgVal in https://bugs.unrealircd.org/view.php?id=6606
My BuildBot screen was also all red :D.
This gets rid of src/buildmod and unifies the process a little, which
i need later.
We still compile the 3rd party modules unconditionally and twice (during
both make and make install). Which is a quirk that is in there since U6
and maybe U5 already :D. That's because we don't check if header files
have changed. There was previously a "is the .c file newer than the .so"
in there, though, that is gone now. Anyway, that's something for later.
Another quirk is that we do not halt compile if a 3rd party module fails
to compile. Which was sortof intentional at one point but.. is not ideal,
so will probably changed as well.
Anyway, that's not why i am doing all this stuff right now...
This is to guard against clients that do like CAP LS 302, NICK, USER,
AUTHENTICATE, CAP END, without waiting for the SASL result.
Previously "CAP END" would abort SASL if the response was not in yet.
Now "CAP END" will cause us to wait for SASL success/fail/timeout
and when that happens we will end the handshake and the user will
come online (or not, if e.g. banned).
In other words, SASL is no longer canceled upon premature CAP END.
And yeah, clients should wait, as is mentioned in
https://ircv3.net/specs/extensions/sasl-3.1
"it is RECOMMENDED to only send CAP END when the SASL exchange is
completed or needs to be aborted"
But since it is a recommendation and not a hard requirement, we'll
be nice and handle this situation server-side.
Of course, clients could still misbehave then by sending stuff
blindly after CAP END, like JOIN events, without even checking
if they got numeric 001 and so on... so in that sense it shifts
the problem a bit.. but.. at least that type of waiting is
hopefully more common :D
Set known_users=NULL during a very limited period, just to be safe.
(Note that it can also be NULL during initial boot, which is a
longer period, which is why we always NULL-check in the code that
accesses it, but this aside)
threshold.
* Possible transition to known-users:
* - logged in is already handled by HOOKTYPE_ACCOUNT_LOGIN so we don't care about those
* - score reached (or just over) the minimum reputation score
* Caveat: if having multiple connections from the same IP then
* the first one may theoretically not have crossed in some cases.
* Ah well, it is a cache, not some precise thingy.
or elsewhere. I don't think this is an actual problem, but at least the
fix from 1abf73309a was inconsistent,
if we check for b->client further down, then we should not be reading
from it a few lines up. As said, don't think this code is reached in
practice, but hey...
the config file, without having to resort to things like mask %~asn:XXX;
Now you can just use:
ban user {
asn { 11111; 22222; 33333; 44444; }
soft yes;
reason "This ASN is not allowed. If you have an account you can still bypass";
}
Requested by nobody but sounds like a good idea :)
Bram Matthys