Code UseItem exploit fix (duplicate items) - ALL L2J VERSIONS

[Tryskell]

https://www.youtube.com/watch?v=wvwudQvQwIE

 

The video was produced using my pack (or so state the server owner/developer), but L2J got the same issue. All non-customized L2J versions using WeaponEquipTask got this issue. Even if you patched other places to avoid to get multiple similar items with same objectId, this is the initial problem and the only fix you should apply.

 

For the goodness of L2J, I decided to post this fix for free.

 

Short version : Upon UseItem use, and if you're currently attacking, WeaponEquipTask is called to delay the time your weapon is wear.

 

Issue : There is no check upon task call. The item is equipped, no matter what happened between the call time, and the wear time (if you deleted, crystallized, dropped, put item elsewhere,...).

 

Fix : check upon task call if the item is still existing on inventory.

 

For L2J (consider to edit the called method - remove item parameter).

 

   /** Weapon Equip Task */
    private static class WeaponEquipTask implements Runnable
    {
        private final L2PcInstance activeChar;
        
        protected WeaponEquipTask(L2PcInstance character)
        {
            activeChar = character;
        }
        
        @Override
        public void run()
        {
            // Check if the item is still on inventory.
            final ItemInstance item = activeChar.getInventory().getItemByObjectId(_objectId);
            if (item == null)
                return;
            
            // Equip or unEquip
            activeChar.useEquippableItem(item, false);
        }
    }

 

If you use aCis, here's the modified task found on UseItem (will be part of rev 380) :

 

			if (activeChar.isAttackingNow())
				ThreadPool.schedule(() -> {
					final ItemInstance itemToTest = activeChar.getInventory().getItemByObjectId(_objectId);
					if(itemToTest == null)
						return;
					
					activeChar.useEquippableItem(itemToTest, false);
				}, activeChar.getAttackEndTime() - System.currentTimeMillis());
			else
				activeChar.useEquippableItem(item, true);

 

Good luck everyone ! 

Edited February 6, 2019 by Tryskell

[Toaka]

Great Job!

[Kara]

Tryskell can i have your kids? 

Regards.

[miscer7]

So this bug could be existing up to this day to a good portion of java servers? If that's the case I'm kinda buffled how it wasn't found/reported earlier..

[Tryskell]

2 hours ago, miscer7 said:

So this bug could be existing up to this day to a good portion of java servers? If that's the case I'm kinda buffled how it wasn't found/reported earlier..

 

All L2J servers with unedited WeaponEquipTask, since WeaponEquipTask exists (added at Gracia 1, previous chronicles haven't it). When it has been first added, there was a check regarding isAttackingNow(), which has been deleted since.

 

I tested adding it back to see if it was impacting things, the exploit is still working.

 

People probably made some workarounds, like unallowing non pet stuff, or disabling the pet inventory. My version handles all cases, it simply denies the problem from the root. There's probably more than pet inventory - if you're fast enough, you can drop an untargetable weapon at your feet.

[Red-Hair-Shanks]

 private final L2PcInstance activeChar;

i thought L2Pcinstance was renamed to Player 

nice job tryskell :D

[Psyancy]

Just now, Red-Hair-Shanks said:

 private final L2PcInstance activeChar;

i thought L2Pcinstance was renamed to Player 

nice job tryskell :D

The second code is for acis you dumbass xD 

[SweeTs]

35 minutes ago, Red-Hair-Shanks said:

 private final L2PcInstance activeChar;

i thought L2Pcinstance was renamed to Player 

nice job tryskell :D

 

34 minutes ago, Designatix said:

The second code is for acis you dumbass xD 

Muhehehehe 

 

[Tryskell]

2 hours ago, Red-Hair-Shanks said:

 private final L2PcInstance activeChar;

i thought L2Pcinstance was renamed to Player 

nice job tryskell :D

 

Genius.

[Sdw]

Thanks for sharing, bloody legacy code. Even if it's just visual on client for unity as item isn't dup server side, it requires fixing.

Edited February 7, 2019 by Sdw

[valanths1990]

same check must be applied if character "crash" and thread is still running will end up with NPE

[zemaitis]

These fixes dont work, was still able to dupe the item (just took few more tries instead of doing it in first or second attempt)

 

Tryskell did you test your fix?

 

And can you put whole UseItem.java so we could see the manner your proposed fix is applied?

Edited February 9, 2019 by zemaitis

[melron]

7 hours ago, zemaitis said:

These fixes dont work, was still able to dupe the item (just took few more tries instead of doing it in first or second attempt)

 

Tryskell did you test your fix?

 

And can you put whole UseItem.java so we could see the manner your proposed fix is applied?

About the code is the correct one and it's working just read the lines and put some logic there. When the time will come to equip the weapon, it will first check if exists in the inventory. There are two reasons if it doesn't work for you.

1) you didn't add it properly 

2) a kind of problem in your thread pool?

[Tryskell]

16 hours ago, zemaitis said:

These fixes dont work, was still able to dupe the item (just took few more tries instead of doing it in first or second attempt)

 

Tryskell did you test your fix?

 

And can you put whole UseItem.java so we could see the manner your proposed fix is applied?

 

I put everything needed in first post, and even explained in detailed how and why it works. It's a 2 lines edit, come on. Imagine you remove isCastingNow() check upon casting skill, that's basically the same.

 

And no, I never test any of my shares. I just hit the keyboard blindfolded, hoping something cool happens. That's why aCis is so cool, for example. But you already know it, annoying orange.

 

[zemaitis]

1 hour ago, Tryskell said:

 

I put everything needed in first post, and even explained in detailed how and why it works. It's a 2 lines edit, come on. Imagine you remove isCastingNow() check upon casting skill, that's basically the same.

 

And no, I never test any of my shares. I just hit the keyboard blindfolded, hoping something cool happens. That's why aCis is so cool, for example. But you already know it, annoying orange.

 

 

I am not sure how to apply the fix (tried multiple times) but the bug is still here. My friend tried to apply it on his server and faced the same issue.

 

I created a repository showing how I apply the fix: (I'm using older rev, so the way I call ThreadPool is different, maybe that's the issue?)

 

https://github.com/eurbon/acis_bugfix

 

 

 

 

Edited February 10, 2019 by zemaitis