Emulator Issues #12019
Improper handling of null bytes in encrypted setting.txt data
Any game that tries to read stuff from the setting.txt
What's the problem? Describe what went wrong.
Take a look at the file
Source/Core/Common/SettingsHandler.cpp, at the function
str is a pointer to the encrypted data, and the data uses a XOR shuffle with a rotating key.
Right at the beginning of the function is the while condition
*str != 0 - which is true at the end of the file, but it's also true when there is a NULL byte in the middle of the encrypted data, which can happen - then, the decryption stops in the middle of the file and the decrypted data is incomplete.
See my comment on pull request 8673 for more information: https://github.com/dolphin-emu/dolphin/pull/8673#issuecomment-601696961
What steps will reproduce the problem?
Find a setting.txt that contains a nullbyte, decrypt using this function, notice that the decryption is wrong.
If necessary, I can provide such a file
Is the issue present in the latest development version? For future reference, please also write down the version number of the latest development version.
Is the issue present in the latest stable version?
Probably, but I can't test that since 5.0-stable doesn't work properly for me.
#2 Updated by Leseratte10 2 months ago
I just tested replacing the condition with an endless loop, and it seems to work properly.
The decrypted data isn't null-terminated, which means, there will be a bunch of crap at the end of the decrypted data (and I don't see a way to detect that). But it does end with a newline.
And because the GetValue function starts from the beginning and returns the first match found, a bunch of additional junk data at the end of the string shouldn't matter.
- Fixed in set to 5.0-11803
- Status changed from Fix pending to Fixed
#6 Updated by Leseratte10 2 months ago
Unfortunately I have to ask you to open this issue again...
The three NANDs I imported yesterday to test the PR (which decrypts the whole buffer) worked perfectly, but now I found another NAND that results - what I first thought - in the same bug. But it's not actually the same bug:
I added a bunch of debug logging output to Dolphin, and everywhere in Dolphin's code the setting.txt is fine, non-corrupted, as it should be after all the fixes merged in the last few PRs. However the serial number the game sends to Wiimmfi was corrupted yet again, even though I saw nothing in the logs that could explain this.
Then I looked at how Dolphin sends the setting.txt or the data within to the game. Turns out Dolphin just stores the whole encrypted file at 80003800, and the game reads it from there and decrypts it on its own.
So far, so good.
HOWEVER, Nintendo actually made the same mistake that was in the setting.txt parsing code in Dolphin (can it really be labelled a "mistake" then if Nintendo does the same? :P ). The binary data that Dolphin copies to 80003800 is a perfectly valid setting.txt - and when I decode it manually, I get the proper content back. But when the GAME decodes the setting.txt (if anyone wants to look at the code, the related function "__SCF1" is in Mario Kart Wii PAL at address 801b2234), it ALSO assumes that a null byte in the encrypted data means "end of file" - the exact same wrong assumption Dolphin made.
So why did this bug (assuming 00 = eof) not affect real consoles, but only Dolphin?
Simple: Nintendo knew the bug, but it was probably too late to fix it.
What did they do? They made sure that if their generated setting.txt for a console would have included a NULL byte in the generated code, they added an arbitrary additional LF (line feed) character before the line, making sure the encrypted data looks "different" and that there is no NULL byte in the encrypted data. This is the explaination for the arbitrary additional line feeds that required the PR 8670: https://github.com/dolphin-emu/dolphin/pull/8670
What this means for Dolphin:
When Dolphin creates a setting.txt from scratch, it needs to make sure that the encrypted data doesn't contain a NULL byte. If it does, the plaintext data needs to be modified (by adding useless line feeds) and re-encoded until the encrypted data no longer contains a NULL byte.
That took me a while to figure out again ...
I'll see if I can somehow fix this issue in Dolphin easily, right now the encryption seems to be done in a kind of stream (every incoming byte is immediately encrypted), which doesn't allow this kind of modification. That means I would need to rewite that part so that the payload data is first collected in decrypted format, and then afterwards it can be encoded.
What a stupid mistake...
#15 Updated by Leseratte10 2 months ago
However what could happen would be that not adding a LF causes a null byte in a certain line, and adding a LF causes a null byte at another point in the same line. So it could still be possible that multiple LFs are needed, until there is no NULL byte in the whole data for the line, couldn't it?