search

inwc3 / JMPQ3

Native Java mpq archive library
Apache License 2.0
38 stars 17 forks source link

Fixed "invalid" hash sizes (map protection?) in Broodwar maps #32

Closed Bytekeeper closed 4 years ago

Bytekeeper commented 4 years ago

Not sure about Warcraft, but some Starcraft maps use the upper 4 bit to make loading the mpq impossible. (The upper 4 bit are dropped because of * 16 in the game/hashtable - but JMPQ3 uses the original hashSize value to create the table)

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.7%) to 85.586% when pulling e62418983e984472b07b1e05938f8e7f5960bc7f on Bytekeeper:master into 70e8e86ede2c93e8451ad3bcc3462b5bcebeda43 on inwc3:master.

Frotty commented 4 years ago

Cool, thanks for the PR 👍 Could you upload a starcraft brood war test map/mpq for this case? (ideally small size with all data removed). And I think the current change has no effect - did you test this? IntelliJ reports it as superfluous:

I think it should be & 0xFFFFFFFFL as the other above, for which it would need to be a long?

@DrSuperGood do you know something about this?

Bytekeeper commented 4 years ago

@Frotty I'm puzzled, why does it show 8 Fs for you? Should be 7 (it even has the prefix 0x0... in my commit) :) Will try to find a small example.

Frotty commented 4 years ago

Oops, I must have mistakenly added one during copy+paste as I didn't check out the branch 😬 Alright, so it seems good. Any thoughts about the test map?

Bytekeeper commented 4 years ago

I have a test map, but it makes the "testDuplicatePaths" test fail. Ok if I skip that map (it's a SC map after all) - or do you want a failing test ;)

java.nio.BufferOverflowException
    at systems.crigges.jmpq3.MpqFile.writeFileAndBlock(MpqFile.java:346)
    at systems.crigges.jmpq3.JMpqEditor.close(JMpqEditor.java:811)
    at systems.crigges.jmpq3.JMpqEditor.close(JMpqEditor.java:705)
    at systems.crigges.jmpq3.JMpqEditor.close(JMpqEditor.java:701)
    at systems.crigges.jmpq3test.MpqTests.testDuplicatePaths(MpqTests.java:252)
Frotty commented 4 years ago

I don't know which test you are referring to with "testDuplicatePaths". Of course there should not be a failing test 😄 . Why exactly is it failing?

Bytekeeper commented 4 years ago

Uhm, check the trace above. The test is systems.crigges.jmpq3test.MpqTests#testDuplicatePaths. I do not know why it fails. I would guess it is just one further "protection" to make editing the map fail. (Besides having an invalid hash size no (listfile) etc etc.)

Frotty commented 4 years ago

Okay I wasn't on latest master on this machine. 😅 So this PR makes it readable but not modifiable? If only that test fails, which seems weird, I suppose you could omit the starcraft map there.

e: and yea, this looks like the usual bugs with non trivial protection.

Bytekeeper commented 4 years ago

Not sure, maybe it is modifiable. I think that would need further analysis. My use case really only needs reading for now. I'll add the map as an exception.

Frotty commented 4 years ago

Nice 👍
What is your use case? Are you using jmpq in some project. Gonna merge.

Bytekeeper commented 4 years ago

@Frotty Yeah - I'm part of a project to enable players to play vs custom AIs for Starcraft: for this project https://makingcomputerdothings.com/schnail-starcraft-human-n-ai-league-development-roadmap-extra-patreon-rewards/ I'm using it to load maps and render a preview inside the client part. Many maps in SC are modified to prevent alteration and make it crash in an editor. Things like the invalid hash size which work in-game but cannot be loaded by an editor.

Frotty commented 4 years ago

Sounds cool 😄 There are also many protections like this for wc3 maps, but I'm not too interested in supporting them. Please don't forget to star the repo if you deem it useful in your projects. Cheers 🍻