diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.06k stars 793 forks source link

Detect hacked items #3813

Closed ukos-git closed 1 year ago

ukos-git commented 2 years ago

The hellfire save game in the attached save game contains a "dragon's ring" which is a hacked item and to some extend is treated as a staff. It might be worth it to invalidate such nonsensical items.

crash.zip

note this was adapted from a previous issues, hence the unrelated comments below

qndel commented 2 years ago

@ukos-git can u upload the character that crashes?

AJenbo commented 2 years ago

Also could you let us know if your are using any translations in the game?

ukos-git commented 2 years ago

No localization on both parties. I may have had de activated on the char some time ago. I added char.tar.gz and diablo.ini from the crashing party. EDIT: wrong upload. The crash may also get caused by the char on the other PC that is not crashing. Both chars use the same mpq and git master.

The staff on the crashing character is a starting item. Assuming that this item causes the crash, can I print more details on the item with gdb?

#9  0x0000000000618469 in LanguageTranslate[abi:cxx11](char const*) ()
#10 0x00000000004d3457 in devilution::(anonymous namespace)::GetStaffPower(devilution::Item&, int, int, bool) ()

I will create two new characters to see if the problem persists.

qndel commented 2 years ago

char.zip Reuploaded in a more standard format

qndel commented 2 years ago

your character doesn't show up for me

glebm commented 2 years ago

From the backtrace, it seems that either AllItemsList[item.IDidx].iName or AllItemsList[item.IDidx].iSName is invalid (these are the only direct calls to LanguageTranslate in that function).

https://github.com/diasurgical/devilutionX/blob/97a5609cfff6b170ff943537e88cbbfbd9304b10/Source/items.cpp#L1131-L1132

AJenbo commented 2 years ago

Invalidi item.IDidx values should be filtered out by the unpackplayer function

ukos-git commented 2 years ago

Joining with two new characters works. The crash is caused by an item on the other (non-crashing) character. crash.zip

I have a few items on that char that were dropped by a random person who joined the game a few times after doing runs. At least one of these items seems to break the game for anybody that joins.

qndel commented 2 years ago

Thanks, managed to reproduce the crash.

image image

.iSName of your staff of charged bolt is somehow NULL and passing that to translation crashes. I don't even see a staff of charged bolt on your char xD

ukos-git commented 2 years ago

Well, I remember having one at some point early on. 😂

qndel commented 2 years ago

INFO: UnPackPlayer NAME: Jazreth BODY SLOT: 2 INFO: UNPACK ITEM! INFO: RECREATE ITEM! INFO: SETUP ALL ITEMS INFO: GET STAFF SPELL Short Staff of Charged Bolt Short Staff of Charged Bolt INFO: GET STAFF POWER: Short Staff of Charged Bolt (null) INFO: NULL KEY

So in multiplayer other characters somehow see your right ring slot item as a crashing staff of charged bolt

Yup, confirmed by trashing the ring and everything works fine - what the hell? :D

ukos-git commented 2 years ago

thanks. Confirmed. This "Dragon Ring" probably was non-legit item. Crashing the game at this point is probably not the right way of handling the error. Would it be possible to add a debug message for items like this?

AJenbo commented 2 years ago

Looks like this is fixed now?

image

AJenbo commented 2 years ago

I'm a bit unsure if I did things correctly as I did not see a staff of holy bolt and i'm not sure which of the characters i should be using :shrug:

StephenCWills commented 2 years ago

Could be related to #3956.

AJenbo commented 2 years ago

Yep, that one fixes the crash.