|Anonymous | Login | Signup for a new account||2019-07-22 05:52 UTC|
|My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0003660||Doomseeker||[All Projects] Security||public||2019-06-12 00:41||2019-07-20 06:32|
|Assigned To||Pol M|
|Priority||normal||Severity||crash||Reproducibility||have not tried|
|Target Version||Fixed in Version|
|Summary||0003660: SRB2 plugin: Mangled memory values (runtime crashes)|
Quote from Pol M
Pol M (developer)
This has been addressed. commit
Marking as needs testing
This is still just a quick fix that will work most of the time but not at all times. When programming the plugin I've never assumed that the master server will be malicious - while this is not the case here, it essentially is from Doomseeker's perspective as the packets arrive mangled for some reason. The code should be more defensive to prevent malicious packets from crashing the program - always.
What will happen if the header.length is below the current threshold but still incorrect? This could be easily simulated even on a non-BSD system by hardcoding header.length = 0xffffff.
Pol M (developer)
Now that I think about it, we know the size based on the standard. It's:
QDataStream &operator>>(QDataStream &stream, ServerPayload &server)
DataStreamOperatorWrapper out = DataStreamOperatorWrapper(&stream);
out.readRaw(16); // Header full of zeros.
server.name = Srb2::asciiOnly(out.readRaw(32));
server.room = out.readQInt32();
server.version = out.readRaw(8);
so that'd be 16+16+8+32+4+8=84 #quick maths.
We could define a static and constant value in the ServerPayload class.
edited on: 2019-06-14 04:14
Quote from Zalewa
If you know this to be an issue for other plugins, please report new issue(s) (for every affected plugin). Thanks.
|On the note that this is remote denial of service, do we need to address this as a security issue with a CVE Identifier?|
|(Increasing issue priority per comment 0003660:0020765 above.)|
edited on: 2019-06-21 14:56
The PR is merged here:https://bitbucket.org/Doomseeker/doomseeker/commits/b9a90f1f56e704c5cbeefe83da2f9ce939920278 [^]
SRB2 servers are still being listed correctly (and SRB2Kart incorrectly) on Windows so I assume it works, however I'll put it into "needs to be tested" state just in case if someone wants to verify on the BSD platform where the problems were first encountered.
As far as CVE goes - that's bureaucracy and I don't feel like doing it.
Mangled packet problems that may or may not exist in other areas of the program are out of scope for this issue.
Pol M (developer)
|Just to make this clear, this issue has nothing to do with the srb2kart player packages, the srb2kart playerlist is still brocken. It's an issue on their, it will be fixed in the next release and has already been patched.|
|Of course, by stating that SRB2Kart still works incorrectly I meant that this is expected. :)|
Quote from Zalewa
I feel like giving it a try. Tracked as 0003665.
It would be preferable to have a version bump for the SRB2 engine plugin, I'd guess.
|Can you reproduce the issue in OP with compiler optimizations turned off (-O0 flag)? (I believe CMake's debug builds also disable optimizations.)|
|Only registered users can voice their support. Click here to register, or here to log in.|
|Supporters:||No one explicitly supports this issue yet.|
|Opponents:||No one explicitly opposes this issue yet.|
|2019-06-12 00:41||WubTheCaptain||New Issue|
|2019-06-12 00:41||WubTheCaptain||Reporter||WubTheCaptain => Pol M|
|2019-06-12 00:42||WubTheCaptain||Status||new => acknowledged|
|2019-06-12 00:43||WubTheCaptain||Relationship added||related to 0003499|
|2019-06-12 20:27||Pol M||Note Added: 0020755|
|2019-06-12 20:27||Pol M||Assigned To||=> Pol M|
|2019-06-12 20:27||Pol M||Status||acknowledged => needs testing|
|2019-06-13 13:24||WubTheCaptain||Target Version||=> 1.3|
|2019-06-13 13:29||WubTheCaptain||Priority||low => normal|
|2019-06-13 13:29||WubTheCaptain||Severity||minor => crash|
|2019-06-13 14:51||Zalewa||Note Added: 0020756|
|2019-06-13 17:23||Pol M||Note Added: 0020762|
|2019-06-14 04:00||WubTheCaptain||Assigned To||Pol M => Zalewa|
|2019-06-14 04:00||WubTheCaptain||Status||needs testing => needs review|
|2019-06-14 04:13||WubTheCaptain||Note Added: 0020764|
|2019-06-14 04:14||WubTheCaptain||Note Edited: 0020764||View Revisions|
|2019-06-14 04:16||WubTheCaptain||Note Added: 0020765|
|2019-06-18 23:56||WubTheCaptain||Note Added: 0020769|
|2019-06-18 23:56||WubTheCaptain||Priority||normal => high|
|2019-06-21 14:56||Zalewa||Note Added: 0020790|
|2019-06-21 14:56||Zalewa||Status||needs review => needs testing|
|2019-06-21 14:56||Zalewa||Note Edited: 0020790||View Revisions|
|2019-06-21 15:05||Pol M||Note Added: 0020792|
|2019-06-21 15:06||Zalewa||Note Added: 0020793|
|2019-06-21 21:11||WubTheCaptain||Relationship added||parent of 0003665|
|2019-06-21 21:13||WubTheCaptain||Note Added: 0020795|
|2019-06-21 21:16||WubTheCaptain||Category||Bug => Security|
|2019-06-21 22:43||WubTheCaptain||Priority||high => normal|
|2019-06-22 10:47||Zalewa||Assigned To||Zalewa => Pol M|
|2019-06-22 10:47||Zalewa||Status||needs testing => assigned|
|2019-06-22 10:47||Zalewa||Status||assigned => needs testing|
|2019-06-28 08:27||WubTheCaptain||Note Added: 0020842|
|2019-06-28 08:27||WubTheCaptain||Status||needs testing => feedback|
|2019-07-20 06:32||Zalewa||Target Version||1.3 =>|
Questions or other issues? Contact Us.
|Copyright © 2000 - 2019 MantisBT Team|