[MC-200000] Merchant trade select packet (C2S) does not check for negative indices

A merchant trade select packet sent from the client to the server is not checked for an index that is less than 0, only less than the size of the trade offer list. Because of this, sending a negative index such as -1 will cause an ArrayIndexOutOfBoundsException to be thrown and logged to the console.

[12:00:00] [Server thread/FATAL] (Minecraft) Error executing task on Server
java.lang.ArrayIndexOutOfBoundsException: null

Code analysis (Yarn mappings):

public void onMerchantTradeSelect(SelectMerchantTradeC2SPacket packet) {
	NetworkThreadUtils.forceMainThread(packet, this, (ServerWorld) this.player.getServerWorld());
	int tradeId = packet.getTradeId();

	ScreenHandler screenHandler = this.player.currentScreenHandler;
	if (screenHandler instanceof MerchantScreenHandler) {
		MerchantScreenHandler merchantScreenHandler = (MerchantScreenHandler) screenHandler;
		
		// Check for i >= 0 before calling the screen handler method(s)
		merchantScreenHandler.setRecipeIndex(tradeId);
		merchantScreenHandler.switchTo(tradeId);
	}
}

Comments 8

onnowhere 2020-09-11T03:09:00Z

██████╗  ██████╗  ██████╗  ██████╗  ██████╗  ██████╗ 
╚════██╗██╔═████╗██╔═████╗██╔═████╗██╔═████╗██╔═████╗
 █████╔╝██║██╔██║██║██╔██║██║██╔██║██║██╔██║██║██╔██║
██╔═══╝ ████╔╝██║████╔╝██║████╔╝██║████╔╝██║████╔╝██║
███████╗╚██████╔╝╚██████╔╝╚██████╔╝╚██████╔╝╚██████╔╝
╚══════╝ ╚═════╝  ╚═════╝  ╚═════╝  ╚═════╝  ╚═════╝ 
                                                     
                                 .''.
       .''.             *''*    :_\/_:     . 
      :_\/_:   .    .:.*_\/_*   : /\ :  .'.:.'.
  .''.: /\ : _\(/_  ':'* /\ *  : '..'.  -=:o:=-
 :_\/_:'.:::. /)\*''*  .|.* '.\'/.'_\(/_'.':'.'
 : /\ : :::::  '*_\/_* | |  -= o =- /)\    '  *
  '..'  ':::'   * /\ * |'|  .'/.\'.  '._____
      *        __*..* |  |     :      |.   |' .---"|
       _*   .-'   '-. |  |     .--'|  ||   | _|    |
    .-'|  _.|  |    ||   '-__  |   |  |    ||      |
    |' | |.    |    ||       | |   |  |    ||      |
 ___|  '-'     '    ""       '-'   '-.'    '`      |____

Nixinova 2020-09-11T03:19:26Z

🎉 Minecraft's had over 200K reported bugs now; congrats?

galaxy_2alex 2020-09-11T04:03:36Z

Please keep messages such as these in the dedicated Mojira subreddit post: https://www.reddit.com/r/Mojira/comments/iqitfw/the_minecraft_java_edition_bug_tracker_has_just/

Thanks 🙂

yenseo Jang 2020-09-11T04:35:28Z

200000 Bug reports!

W_V 2020-09-11T05:04:18Z

Only MC-2 and MC-20 were valid bugs, MC-200, MC-2000 and MC-20000 weren't valid

galaxy_2alex 2020-09-11T15:02:58Z

Any further comments that are not related to the bug will be removed - don't get me wrong, I agree with the sentiment - but we have a separated Reddit post for this which I linked above. Thanks!

user-189af 2020-09-11T19:53:58Z

Can someone help me understand the issue to attempt to reproduce?

marcono1234 2020-09-12T21:29:18Z

@@unknown, this is about the packet ServerboundSelectTradePacket (1.16.3, Mojang name), correct?
And after the exception was logged by the server the connection is closed, right? Though the server-side validation of packets should probably be improved nonetheless.