View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006432 | unreal | ircd | public | 2024-07-17 14:25 | 2024-07-17 14:48 |
| Reporter | Valware | Assigned To | |||
| Priority | normal | Severity | tweak | Reproducibility | N/A |
| Status | new | Resolution | open | ||
| Product Version | 6.1.6 | ||||
| Summary | 0006432: Verify upstream issuer's name can be used properly in places on IRC | ||||
| Description | Report by Valware: I made an account on my webpanel with a space in the username just to see what would happen, changed the topic of a channel and things got a bit messy due to explicitly trusting that it can be used on IRC. I'm going to fix that in the webpanel also, but it doesn't stop other devs from potentially breaking things with their own software. So I think we should just do this to be safe. *** Comment by Syzop: I see. While the code may look logical and correct at first sight, the do_nick_name(client->rpc->issuer) will cause the client->rpc->issuer to be altered. And changing client->rpc->issuer from issued-by-tag is rather unexpected (and bad). We should either: * Sanitize and reject the issuer where the RPC call gets done And/or * Sanitize it in issued-by-tag (not the client->rpc->issuer but use a temporary variable so to not touch the original). But, I think I should first look at the underlying problem how big it is. You mention causing a mess but didn't really say what happened exactly. Looking quickly at the code I think it is more that the set_by value is unchecked while it should be (but that is technically unrelated to issuer). Anyway, working on other stuff atm. *** Follow-up by Valware: For example, when my webpanel username is "test username with spaces", and I go to set a topic to "lol", it looks like this: <- @time=2024-07-11T18:44:22.193Z :irc.valware.uk 332 Valware #test :with spaces 1720723428 :lol <- @time=2024-07-11T18:44:22.193Z :irc.valware.uk 333 Valware #test test 1720723428 which gets rendered like this: [02:43:48] * irc2.valware.co.uk changes topic to 'with spaces 1720723428 :lol' | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | |||||
|
|
"Looking quickly at the code I think it is more that the set_by value is unchecked while it should be (but that is technically unrelated to issuer). " Did you check that ? Probably not? |
|
|
Yeah it is what I said and not related to issuer. This also implies that your patch could not possibly fix the issue, which in turn implies that the patch was not tested to fix the issue. Or am I missing something... hmm.... |
|
|
Ah I get it, you can trigger the bug both ways, via issuer and via set_by... as there is a if (set_by == NULL) set_by = client->name.... so your patch fixed one way, and the other way was set_by. We would need to fix both and I also wonder how widespread this is, like in all RPC calls, things that never contain spaces... because then we could/should sanitize them all in the same way :) |
|
|
Ah well I am going to hunt a more serious bug at the moment. Thanks for the report I will look into it later... and fix it... in a general way. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-07-17 14:25 | syzop | New Issue | |
| 2024-07-17 14:26 | syzop | Reporter | syzop => Valware |
| 2024-07-17 14:26 | syzop | Note Added: 0023262 | |
| 2024-07-17 14:44 | syzop | Note Added: 0023264 | |
| 2024-07-17 14:46 | syzop | Note Added: 0023265 | |
| 2024-07-17 14:47 | syzop | Note Edited: 0023265 | |
| 2024-07-17 14:48 | syzop | Note Added: 0023266 |
