Fix rock-paper-scissors response validation#846
Conversation
|
confirming |
| if(repeat) { | ||
| pduel->write_buffer8(MSG_ROCK_PAPER_SCISSORS); | ||
| pduel->write_buffer8(0); | ||
| core.units.begin()->step = 0; |
There was a problem hiding this comment.
Although step is an unsigned integer, the codebase already contains many cases where -1 is used to indicate starting from the beginning, so I would not recommend changing it.
There was a problem hiding this comment.
it sent MSG_ROCK_PAPER_SCISSORS again so it should go to 1 instead
| pduel->write_buffer8(1); | ||
| return FALSE; | ||
| } | ||
| case 2: { |
There was a problem hiding this comment.
All operations in this file involve only a single player and consist of only two steps. In contrast, Rock-Paper-Scissors involves two players and multiple steps, so adding RPS operation here could be misleading (
Line 73 in 19026e3
There was a problem hiding this comment.
playerop.cpp already indicated it's a player operation
in addition sending MSG_RETRY in processor.cpp seemed weird
There was a problem hiding this comment.
To avoid MSG_RETRY, how about treat 0 as 1, 4 and above as 3 silently?
There was a problem hiding this comment.
it should always be MSG_RETRY if client sends invalid thing
this is the simplest way, and to avoid potential security problems
3 participants
Game-internal rock-paper-scissors previously trusted the raw player response and accepted values outside the valid 1..3 hand range.
That meant invalid responses could be stored as player 0's hand, packed into MSG_HAND_RES, or shifted while resolving player 1's hand. In practice this could produce malformed hand-result messages and let an invalid response affect winner resolution instead of asking the same player to respond again.
This change moves the rock-paper-scissors player-response handling into playerop.cpp and validates each response before using it. Out-of-range values now emit MSG_RETRY and keep the processor on the same rock-paper-scissors step, so the same player must provide a valid hand before resolution continues.
The normal result logic is preserved for valid hands, including repeat ties and PLAYER_NONE for no-repeat ties. Hand values are also cast to uint8_t before packing MSG_HAND_RES.
Inspired by https://github.com/edo9300/ygopro-core/blob/2407cf72000d8155822330d1b3cd08acb7618f4d/playerop.cpp#L1122-L1130