View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0004961 | unreal | ircd | public | 2017-06-03 21:49 | 2017-08-10 08:35 |
| Reporter | Gottem | Assigned To | syzop | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Fixed in Version | 4.0.13 | ||||
| Summary | 0004961: m_tkl ban_too_broad() logic errors | ||||
| Description | I was messing around with a module and noticed sometimes checks inside ban_too_broad() never trigger because the conditions around it contain some errors. Or the function returns an incorrect value. ==== /zline *@8.0.0.0/8 This should be considered too broad, but Unreal happily accepts it. This is due to: for (p = hostmask; *p; p++) if (*p != '*' && *p != '.' && *p != '?') cnt++; if (cnt >= 4) return 0; It doesn't take into account the CIDR notation at the end, and "8000" satisfies the if() so it returns 0 too soon. ==== /zline *@/24 Should be considered incorrect to begin with, but Unreal ends up saying it's "too broad" (earlier if() is false since cnt = 3 now). Except the /24 would actually be not-too-broad. This is due to: if (strchr(hostmask, ':')) { if (cidrlen < 48) return 0; /* too broad IPv6 CIDR mask */ } else { if (cidrlen < 16) return 0; /* too broad IPv4 CIDR mask */ A cidrlen of /8 is indeed broader than /24, so it should return 1 here (same goes for IPv6). Alternatively, reverse only the conditions and do cidrlen >= 48 and cidrlen >= 16 instead. ==== /zline *@8.0.0.0/58963 Happily accepted as well. I noticed _match_user() does check for the validity of the cidrlen, but it doesn't throw a visible error. It just returns "nomatch" silently whenever a user is checked against tklines. ==== Bonus? /gline *@8.8.8.0/24 Hmm, a G:Line with a CIDR mask, seems like that should be rejected. :> | ||||
| Additional Information | Quite a wall of text, hope "you can make chocolate out of it" once I press submit. =] | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | N/A | ||||
|
|
Thanks, fixed! :) https://github.com/unrealircd/unrealircd/commit/18202a0f73ef8f32cc8cbf9c86a761740f2bbfb1 ( As for the setting of invalid-never-matching-bans... I'm OK with leaving that in for now, just like with +b.. it simply won't match. ) |
|
|
By the way, just FYI: a GLINE with a CIDR mask is OK. You can use a hostname or IP (or CIDR) in GLINE. The difference between a GLINE/KLINE and GZLINE/ZLINE is that the latter is enforced immediately after connect() before accepting any data. While the former only after USER and NICK have been received.. and a possible SSL/TLS handshake has been finished. It can still be useful to use IP's in GZLINE/ZLINE because otherwise SSL/TLS users may not see the ban reason, although this may depend on the client. ^^ should probably document that better somewhere... but where. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2017-06-03 21:49 | Gottem | New Issue | |
| 2017-08-10 08:32 | syzop | Assigned To | => syzop |
| 2017-08-10 08:32 | syzop | Status | new => resolved |
| 2017-08-10 08:32 | syzop | Resolution | open => fixed |
| 2017-08-10 08:32 | syzop | Fixed in Version | => 4.0.13 |
| 2017-08-10 08:32 | syzop | Note Added: 0019810 | |
| 2017-08-10 08:35 | syzop | Note Added: 0019811 | |
| 2017-08-10 08:35 | syzop | Note Edited: 0019811 | |
| 2017-08-10 08:35 | syzop | Note Edited: 0019811 |
