View Issue Details

IDProjectCategoryView StatusLast Update
0005934unrealircdpublic2021-07-03 14:11
Reporterpuckipedia Assigned Tosyzop  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version5.2.0.1 
Fixed in Version5.2.1-rc1 
Summary0005934: SJOIN bursts not properly propagated to SJSBY-supporting clients
DescriptionWhen an SJSBY burst is built and overflows in cmd_sjoin, due to a copy-paste error(?) it accidentally sends the SJSBY burst to non-SJSBY supporting clients. Of course, these overflows should usually not happen. But due to a difference in maximum line length between `server.c` generating the initial SJOIN and cmd_sjoin recreating this burst, completely full SJOIN bursts, in some easily reproducible situations, will be discarded entirely.

This has been determined due to pissnet's interesting architecture, and tooling by gerard (on pissnet) that sweeped the entire network for mismatches in channel information, on which it was visible that, while a single node was aware of a nick being joined, all the other nodes connecting to that node were not aware, leading to going through the SJOIN code with a fine comb.

Applied is a lightly-tested patch that seems to fix this behavior.
Steps To ReproduceSet up three UnrealIRCD servers, linking them as `A -> B -> C`.

- Connect many clients (i tested with 100) to server A, have them all join "#tests" (channel length might matter)
- From B, SQUIT A. This will netsplit everyone off.
- Watch the traffic received by B (from A) versus what B sends to C.
Additional InformationDebug log from my test:

Parsing: @msgid=gbw8qFlzan6jQNHNC519pC;time=2021-06-26T22:56:10.391Z :001 SJOIN 1624747860 #tests +nt :0013HK61J 001FHEG1I 001BIZV1H 001C8YZ1G 001MJ3M1F 001XKIB1E 001A3WO1D 0011BQ81C 0016F1D1B 001XAK21A 001L9ZW19 001C5QU18 001TE9017 001EM7P16 001LNTH15 0019CK914 001ZNI913 0017OPC12 001XGDP11 001DP1510 001AJ1R0Z 001YH2Q0Y 001UZON0X 001QG7S0W 001F55K0V 001G8GP0U 001EBS70T 001CBCZ0S 001B2OR0R 0019LGK0Q 001M5VG0P 001XXPN0O 0013TF30N 001N89K0M 001RXQ20L 00100VI0K 001ATKW0J 001ST8Z0I 00124NJ0H 001BVR70G 001KF0H0F 001FY530E 00196OS0D 001X5SS0C 001CVPO0B 0013SQJ0A 001E3ES09 (from one.example.org)
Sending [@time=2021-06-26T22:56:10.391Z :user52!user52@Clk-66742DE9 JOIN #tests * :user52] to puckipedia_
[...]
Sending [@time=2021-06-26T22:56:10.391Z :user6!user6@Clk-66742DE9 JOIN #tests * :user6] to puckipedia_
Sending [@time=2021-06-26T22:56:10.391Z;msgid=gbw8qFlzan6jQNHNC519pC :001 SJOIN 1624747860 #tests +nt :001E3ES09 ] to three.example.org

Post-applying the patch:

Parsing: @msgid=P0pgnTEjMB2ukmKkpbDd3i;time=2021-06-26T23:07:57.312Z :001 SJOIN 1624748724 #tests +nt :001XHKE03 001QCKW2V 001JX3L2U 001B3II2T 001N3Q32S 001R84U2R 001FOW22Q 001733O2P 001DGCL2O 001MFDD2N 001Y38C2M 001H4ED2L 001ZJM42K 001YCM82J 001MMMR2I 001Y12U2H 001R0ZH2G 001ASZ72F 0013U4U2E 001HAJR2D 001WFV42C 001L5GV2B 001ZM6O2A 0019BFW29 001QNZN28 001RA5V27 0019XFU26 001PWZF25 001S7GE24 0012JHA23 001UE8022 001LI0921 0018QPX20 001A0SO1Z 0015C1X1Y 0010Z5P1X 001X95U1W 001XXYV1V 001EH1J1U 001LC321T 001RLZL1S 001L3WV1R 001XWL01Q 001HGUJ1P 001IJQX1O 001TIJI1N 0011WRH1M (from one.example.org)
Sending [@time=2021-06-26T23:07:57.312Z;msgid=P0pgnTEjMB2ukmKkpbDd3i :001 SJOIN 1624748724 #tests +nt :001XHKE03 001QCKW2V 001JX3L2U 001B3II2T 001N3Q32S 001R84U2R 001FOW22Q 001733O2P 001DGCL2O 001MFDD2N 001Y38C2M 001H4ED2L 001ZJM42K 001YCM82J 001MMMR2I 001Y12U2H 001R0ZH2G 001ASZ72F 0013U4U2E 001HAJR2D 001WFV42C 001L5GV2B 001ZM6O2A 0019BFW29 001QNZN28 001RA5V27 0019XFU26 001PWZF25 001S7GE24 0012JHA23 001UE8022 001LI0921 0018QPX20 001A0SO1Z 0015C1X1Y 0010Z5P1X 001X95U1W 001XXYV1V 001EH1J1U 001LC321T 001RLZL1S 001L3WV1R 001XWL01Q 001HGUJ1P 001IJQX1O 001TIJI1N ] to three.example.org
Sending [@time=2021-06-26T23:07:57.312Z;msgid=P0pgnTEjMB2ukmKkpbDd3i :001 SJOIN 1624748724 #tests +nt :0011WRH1M ] to three.example.org
TagsNo tags attached.
3rd party modules

Activities

puckipedia

2021-06-27 01:18

reporter  

sjsby.patch (1,043 bytes)

gerard

2021-06-27 01:58

reporter   ~0022035

Would it be possible to increase the sjoin module version ("5.0" -> "5.0.1" or similar) if this patch is accepted and applied? Being able to remotely check if a server has this fix applied would be very helpful, even if they only update/hotfix the sjoin module.

syzop

2021-06-27 07:42

administrator   ~0022036

Last edited: 2021-06-27 07:43

Thanks a lot for the detailed report and the patch.

Indeed, this was caused already when SJSBY was added in 28-jan-2019 https://github.com/unrealircd/unrealircd/commit/874d99e0eb694a3fa9fe985d9ea81e12310ac1a5#diff-805ab977c31ae294ad966279c399b20fa408376b3c833cc2ca8c6322374d453dR569
Exactly what you said.. clearly a copy-paste error.

Fixed in https://github.com/unrealircd/unrealircd/commit/c37c9655069bd481b55f418b9dc3054b52825438
commit c37c9655069bd481b55f418b9dc3054b52825438
Author: Bram Matthys <syzop@vulnscan.org>
Date: Sun Jun 27 07:37:26 2021 +0200

    Fix SJOIN not properly propagated due to a copy-paste error in the SJSBY
    vs non-SJSBY code. Reported by puckipedia in
    https://bugs.unrealircd.org/view.php?id=5934


Oh.. and I only read the command from gerard later. Did the version bump in a later commit, though.

syzop

2021-06-27 07:52

administrator   ~0022037

You can now hot-patch this without restart via: ./unrealircd hot-patch sjoinsjsby
Patch works on both 5.0.x and 5.2.x and bumps the version of the sjoin module to 5.1.

syzop

2021-06-27 09:25

administrator   ~0022038

Test framework (that runs on each commit via buildbot) now has a test which puts 75 clones on 3 servers, then squits and (re)connects them, checks for missing users.
The test fails before applying the sjoin patch and succeeds after.
I will expand the tests later with more, like syncing +beI and +vhoaq and the different cases of "merge" vs "loosing/winning side".
Was already planned to do "some day" but done so now since it is not acceptable that this bug was overlooked for so long.

Issue History

Date Modified Username Field Change
2021-06-27 01:18 puckipedia New Issue
2021-06-27 01:18 puckipedia File Added: sjsby.patch
2021-06-27 01:58 gerard Note Added: 0022035
2021-06-27 07:42 syzop Note Added: 0022036
2021-06-27 07:43 syzop Note Edited: 0022036
2021-06-27 07:43 syzop Note Edited: 0022036
2021-06-27 07:52 syzop Note Added: 0022037
2021-06-27 09:25 syzop Note Added: 0022038
2021-07-03 14:11 syzop Assigned To => syzop
2021-07-03 14:11 syzop Status new => resolved
2021-07-03 14:11 syzop Resolution open => fixed
2021-07-03 14:11 syzop Fixed in Version => 5.2.1-rc1