[Rseding91] [15.19] Map exchange strings need better validation

This subforum contains all the issues which we already resolved.
D0SBoots
Inserter
Inserter
Posts: 43
Joined: Sat Nov 05, 2016 10:11 pm
Contact:

[Rseding91] [15.19] Map exchange strings need better validation

Post by D0SBoots »

The newly-added "Technology price multiplier" setting at map creation has validation on the input box, but apparently no validation when reading from a map exchange string.

This string sets the price multiplier to 0.1, which makes a lot of achievements pretty trivial to get:

Code: Select all

>>>AAAPABMAAwADAwcAAAAEAAAAY29hbAMDAwoAAABjb3BwZXItb3JlAwMDCQAAAGNydWRlLW9pbAMDAwoAAABlbmVteS1iYXNlAwADCAAAAGlyb24tb3JlAwMDBQAAAHN0b25lAwMDCwAAAHVyYW5pdW0tb3JlAwMDrQZ6hYCEHgCAhB4AAwABAAF7FK5H4XqUPwEAAAAAAAAuQAEAAAAAAADwPwEAAAAAAFi7QAEAAAAAAOCFQAEAAAAAAFirQAEAAAAAAIjDQAEAAAAAAECfQAEAAAAAAEB/QAEAAAAAAECPQAEzMzMzMzPzPwEzMzMzMzPzPwF7FK5H4Xp0PwEAAQAAAAAAAAhAAQAAAAAAAAhAAXsUrkfheoQ/AQABAAGN7bWg98bQPgH8qfHSTWJgPwFpHVVNEHXvPgEAAQcAAAABAgAAAAECAAAAAZqZmZmZmbk/AQAAAAAAAABAAQAAAAAAAOA/AZqZmZmZmdk/Ac3MzMzMzOw/AQUAAAABFAAAAAFAOAAAAcBLAwABEA4AAAGgjAAAASAcAAABAAAAAAAAPkABAAAAAAAAFEABZmZmZmZm9j8BMzMzMzMz4z8BMzMzMzMz0z8BAAAAAAAACEABAAAAAAAAJEABPAAAAAEeAAAAAcgAAAABBQAAAAEAAAAAAAAAQAEBAQAAAAAAAFlAAQUAAAABGQAAAAEAAAAAAAAkQAEyAAAAAQAAAAAAAD5AAWQAAAABmpmZmZmZyT8BMzMzMzMzwz8BMzMzMzMz0z8BMzMzMzMz0z8BAAAAAAAAJEABAAAAAAAANEABAAAAAAAAPkABAAAAAAAAFEABAAAAAAAAPkABAAAAAAAAJEABAAAAAAAACEABCgAAAAFkAAAAAWQAAAAB6AMAAAEAAAAAAADgPwHQBwAAAQAAAAAAQH9AAwAAAAAAmpmZmZmZuT8mOO8h<<<
This string sets the price multiplier to 0, which crashes the game as soon as you try to open the research screen:

Code: Select all

>>>AAAPABMAAwADAwcAAAAEAAAAY29hbAMDAwoAAABjb3BwZXItb3JlAwMDCQAAAGNydWRlLW9pbAMDAwoAAABlbmVteS1iYXNlAwADCAAAAGlyb24tb3JlAwMDBQAAAHN0b25lAwMDCwAAAHVyYW5pdW0tb3JlAwMDrQZ6hYCEHgCAhB4AAwABAAF7FK5H4XqUPwEAAAAAAAAuQAEAAAAAAADwPwEAAAAAAFi7QAEAAAAAAOCFQAEAAAAAAFirQAEAAAAAAIjDQAEAAAAAAECfQAEAAAAAAEB/QAEAAAAAAECPQAEzMzMzMzPzPwEzMzMzMzPzPwF7FK5H4Xp0PwEAAQAAAAAAAAhAAQAAAAAAAAhAAXsUrkfheoQ/AQABAAGN7bWg98bQPgH8qfHSTWJgPwFpHVVNEHXvPgEAAQcAAAABAgAAAAECAAAAAZqZmZmZmbk/AQAAAAAAAABAAQAAAAAAAOA/AZqZmZmZmdk/Ac3MzMzMzOw/AQUAAAABFAAAAAFAOAAAAcBLAwABEA4AAAGgjAAAASAcAAABAAAAAAAAPkABAAAAAAAAFEABZmZmZmZm9j8BMzMzMzMz4z8BMzMzMzMz0z8BAAAAAAAACEABAAAAAAAAJEABPAAAAAEeAAAAAcgAAAABBQAAAAEAAAAAAAAAQAEBAQAAAAAAAFlAAQUAAAABGQAAAAEAAAAAAAAkQAEyAAAAAQAAAAAAAD5AAWQAAAABmpmZmZmZyT8BMzMzMzMzwz8BMzMzMzMz0z8BMzMzMzMz0z8BAAAAAAAAJEABAAAAAAAANEABAAAAAAAAPkABAAAAAAAAFEABAAAAAAAAPkABAAAAAAAAJEABAAAAAAAACEABCgAAAAFkAAAAAWQAAAAB6AMAAAEAAAAAAADgPwHQBwAAAQAAAAAAQH9AAwAAAAAAAAAAAAAAAACHtGuy<<<
This string sets the coal frequency to 0, which results in an empty-appearing dropdown in the settings page but otherwise appears to work correctly. (I.e. no coal spawns.) Other dialog boxes can be set to option 0 in a similar way, but it appears that they are properly guarded against too-large values.

Code: Select all

AAAPABMAAwADAwcAAAAEAAAAY29hbAADBQoAAABjb3BwZXItb3JlAwMDCQAAAGNydWRlLW9pbAMDAwoAAABlbmVteS1iYXNlAwADCAAAAGlyb24tb3JlAwMDBQAAAHN0b25lAwMDCwAAAHVyYW5pdW0tb3JlAwMDrQZ6hYCEHgCAhB4AAwABAAF7FK5H4XqUPwEAAAAAAAAuQAEAAAAAAADwPwEAAAAAAFi7QAEAAAAAAOCFQAEAAAAAAFirQAEAAAAAAIjDQAEAAAAAAECfQAEAAAAAAEB/QAEAAAAAAECPQAEzMzMzMzPzPwEzMzMzMzPzPwF7FK5H4Xp0PwEAAQAAAAAAAAhAAQAAAAAAAAhAAXsUrkfheoQ/AQABAAGN7bWg98bQPgH8qfHSTWJgPwFpHVVNEHXvPgEAAQcAAAABAgAAAAECAAAAAZqZmZmZmbk/AQAAAAAAAABAAQAAAAAAAOA/AZqZmZmZmdk/Ac3MzMzMzOw/AQUAAAABFAAAAAFAOAAAAcBLAwABEA4AAAGgjAAAASAcAAABAAAAAAAAPkABAAAAAAAAFEABZmZmZmZm9j8BMzMzMzMz4z8BMzMzMzMz0z8BAAAAAAAACEABAAAAAAAAJEABPAAAAAEeAAAAAcgAAAABBQAAAAEAAAAAAAAAQAEBAQAAAAAAAFlAAQUAAAABGQAAAAEAAAAAAAAkQAEyAAAAAQAAAAAAAD5AAWQAAAABmpmZmZmZyT8BMzMzMzMzwz8BMzMzMzMz0z8BMzMzMzMz0z8BAAAAAAAAJEABAAAAAAAANEABAAAAAAAAPkABAAAAAAAAFEABAAAAAAAAPkABAAAAAAAAJEABAAAAAAAACEABCgAAAAFkAAAAAWQAAAAB6AMAAAEAAAAAAADgPwHQBwAAAQAAAAAAQH9AAwAAAAAAAAAAAAAA8D+1lMwv
I didn't bother checking any of the other fields, such as trying to set invalid values for the checkboxes or going out-of-bounds for slider values. It'll be easier for you to audit them then for me to fuzz them all manually.
Rseding91
Factorio Staff
Factorio Staff
Posts: 16028
Joined: Wed Jun 11, 2014 5:23 am
Contact:

Re: [Rseding91] [15.19] Map exchange strings need better validation

Post by Rseding91 »

Thanks for the report. I've added checks for the recipe/technology values so the game won't crash but the rest I don't consider worth adding checks for. We don't do validation of the data loaded from save files in any way and assume that it's correct to be loaded so doing the same for exchange strings seems fine to me. If the exchange string or map loaded was changed it then it will either crash or you get what you changed it to.

It's like trying to guard against memory editing the process while it runs - it's just not worth the effort when 99%+ of the users will never do it and the tiny tiny fraction that does won't be stopped by any measures we put in place. All that, and it would bloat the code while taking up developer time that could be better spent on other things.

Basically: don't binary edit the exchange string if you don't want the results you get :P It only "harms" you the person using the edited contents - not something like SQL injection that could leak personal info.
If you want to get ahold of me I'm almost always on Discord.
Post Reply

Return to “Resolved Problems and Bugs”