From 178d1d2bda75e26a9964d2afa2d7fa1210e7f2ea Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Sun, 2 Jan 2022 12:19:13 +0000 Subject: ClientHandle: improve right-click robustness (#5372) * ClientHandle: improve right-click robustness + Add checks for result of GetBlockTypeMeta + Kick if the client sent an invalid block face or coordinate * Update outdated comments --- src/ClientHandle.cpp | 74 +++++++++++++++++++++++++++-------------------- src/Items/ItemHandler.cpp | 35 ++++++---------------- src/Items/ItemHandler.h | 10 +++---- 3 files changed, 56 insertions(+), 63 deletions(-) diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp index 1d9b8e2a6..16c040e49 100644 --- a/src/ClientHandle.cpp +++ b/src/ClientHandle.cpp @@ -1437,38 +1437,45 @@ void cClientHandle::FinishDigAnimation() void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, eBlockFace a_BlockFace, int a_CursorX, int a_CursorY, int a_CursorZ, bool a_UsedMainHand) { - // This function handles three actions: - // (1) Place a block; - // (2) "Use" a block: Interactive with the block, like opening a chest/crafting table/furnace; - // (3) Use the held item targeting a block. E.g. farming. - // - // Sneaking player will not use the block if hand is not empty. - // Frozen player can do nothing. - // In Game Mode Spectator, player cannot use item or place block, but can interactive with some block depending on cBlockInfo::IsUseableBySpectator(BlockType) - // - // If the action failed, we need to send an update of the placed block or inventory to the client. - // - // Actions rejected by plugin will not lead to other attempts. - // E.g., when opening a chest with a dirt in hand, if the plugin rejects opening the chest, the dirt will not be placed. + /* This function handles three actions: + (1) Place a block; + (2) "Use" a block: Interactive with the block, like opening a chest/crafting table/furnace; + (3) Use the held item targeting a block. E.g. farming. + + Sneaking player will not use the block if hand is not empty. + Frozen player can do nothing. + In Game Mode Spectator, player cannot use item or place block, but can interactive with some block depending on cBlockInfo::IsUseableBySpectator(BlockType) + + If the action failed, we need to send an update of the placed block or inventory to the client. + + Actions rejected by plugin will not lead to other attempts. + E.g., when opening a chest with a dirt in hand, if the plugin rejects opening the chest, the dirt will not be placed. */ + + if ((a_BlockFace == BLOCK_FACE_NONE) || !cChunkDef::IsValidHeight(a_BlockY)) + { + LOGD("Player \"%s\" sent an invalid click - hacked client?", m_Username.c_str()); + return; + } // TODO: We are still consuming the items in main hand. Remove this override when the off-hand consumption is handled correctly. a_UsedMainHand = true; + + const Vector3i ClickedPosition(a_BlockX, a_BlockY, a_BlockZ); + const Vector3i CursorPosition(a_CursorX, a_CursorY, a_CursorZ); const cItem & HeldItem = a_UsedMainHand ? m_Player->GetEquippedItem() : m_Player->GetInventory().GetShieldSlot(); - auto & ItemHandler = HeldItem.GetHandler(); - // TODO: This distance should be calculated from the point that the cursor pointing at, instead of the center of the block - // Distance from the block's center to the player's eye height - auto ClickedBlockPos = Vector3i(a_BlockX, a_BlockY, a_BlockZ); - auto CursorPos = Vector3i(a_CursorX, a_CursorY, a_CursorZ); - double Dist = (Vector3d(ClickedBlockPos) + Vector3d(0.5, 0.5, 0.5) - m_Player->GetEyePosition()).Length(); - FLOGD("HandleRightClick: {0}, face {1}, Cursor {2}, Hand: {3}, HeldItem: {4}; Dist: {5:.02f}", - ClickedBlockPos, a_BlockFace, CursorPos, a_UsedMainHand, ItemToFullString(HeldItem), Dist - ); + // Distance from the block's center to the player's eye height. + const double Dist = (Vector3d(0.5, 0.5, 0.5) + ClickedPosition - m_Player->GetEyePosition()).SqrLength(); // Check the reach distance: // _X 2014-11-25: I've maxed at 5.26 with a Survival client and 5.78 with a Creative client in my tests - double MaxDist = m_Player->IsGameModeCreative() ? 5.78 : 5.26; + double MaxDist = m_Player->IsGameModeCreative() ? 33.4084 : 27.6676; bool IsWithinReach = (Dist <= MaxDist); + + FLOGD("HandleRightClick: {0}, face {1}, Cursor {2}, Hand: {3}, HeldItem: {4}; Dist: {5:.02f}", + ClickedPosition, a_BlockFace, CursorPosition, a_UsedMainHand, ItemToFullString(HeldItem), Dist + ); + cWorld * World = m_Player->GetWorld(); cPluginManager * PlgMgr = cRoot::Get()->GetPluginManager(); @@ -1479,11 +1486,16 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e { BLOCKTYPE BlockType; NIBBLETYPE BlockMeta; - World->GetBlockTypeMeta(ClickedBlockPos, BlockType, BlockMeta); - const auto & BlockHandler = cBlockHandler::For(BlockType); - bool Placeable = ItemHandler.IsPlaceable() && !m_Player->IsGameModeAdventure() && !m_Player->IsGameModeSpectator(); - bool BlockUsable = BlockHandler.IsUseable() && (!m_Player->IsGameModeSpectator() || cBlockInfo::IsUseableBySpectator(BlockType)); + if (!World->GetBlockTypeMeta(ClickedPosition, BlockType, BlockMeta)) + { + return; + } + + const auto & ItemHandler = HeldItem.GetHandler(); + const auto & BlockHandler = cBlockHandler::For(BlockType); + const bool Placeable = ItemHandler.IsPlaceable() && !m_Player->IsGameModeAdventure() && !m_Player->IsGameModeSpectator(); + const bool BlockUsable = BlockHandler.IsUseable() && (!m_Player->IsGameModeSpectator() || cBlockInfo::IsUseableBySpectator(BlockType)); if (BlockUsable && !(m_Player->IsCrouched() && !HeldItem.IsEmpty())) { @@ -1491,7 +1503,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e if (!PlgMgr->CallHookPlayerUsingBlock(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ, BlockType, BlockMeta)) { // Use a block: - if (BlockHandler.OnUse(ChunkInterface, *World, *m_Player, ClickedBlockPos, a_BlockFace, CursorPos)) + if (BlockHandler.OnUse(ChunkInterface, *World, *m_Player, ClickedPosition, a_BlockFace, CursorPosition)) { PlgMgr->CallHookPlayerUsedBlock(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ, BlockType, BlockMeta); return; // Block use was successful, we're done. @@ -1501,7 +1513,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e if (Placeable) { // Place a block: - ItemHandler.OnPlayerPlace(*m_Player, HeldItem, {a_BlockX, a_BlockY, a_BlockZ}, a_BlockFace, {a_CursorX, a_CursorY, a_CursorZ}); + ItemHandler.OnPlayerPlace(*m_Player, HeldItem, ClickedPosition, BlockType, BlockMeta, a_BlockFace, CursorPosition); } return; @@ -1518,7 +1530,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e } // Place a block: - ItemHandler.OnPlayerPlace(*m_Player, HeldItem, {a_BlockX, a_BlockY, a_BlockZ}, a_BlockFace, {a_CursorX, a_CursorY, a_CursorZ}); + ItemHandler.OnPlayerPlace(*m_Player, HeldItem, ClickedPosition, BlockType, BlockMeta, a_BlockFace, CursorPosition); return; } else if (!PlgMgr->CallHookPlayerUsingItem(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ)) @@ -1527,7 +1539,7 @@ void cClientHandle::HandleRightClick(int a_BlockX, int a_BlockY, int a_BlockZ, e // Use an item in hand with a target block. cBlockInServerPluginInterface PluginInterface(*World); - ItemHandler.OnItemUse(World, m_Player, PluginInterface, HeldItem, {a_BlockX, a_BlockY, a_BlockZ}, a_BlockFace); + ItemHandler.OnItemUse(World, m_Player, PluginInterface, HeldItem, ClickedPosition, a_BlockFace); PlgMgr->CallHookPlayerUsedItem(*m_Player, a_BlockX, a_BlockY, a_BlockZ, a_BlockFace, a_CursorX, a_CursorY, a_CursorZ); return; } diff --git a/src/Items/ItemHandler.cpp b/src/Items/ItemHandler.cpp index fa518da20..95ee28bff 100644 --- a/src/Items/ItemHandler.cpp +++ b/src/Items/ItemHandler.cpp @@ -1042,30 +1042,15 @@ const cItemHandler & cItemHandler::For(int a_ItemType) -void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, const Vector3i a_ClickedBlockPosition, const eBlockFace a_ClickedBlockFace, const Vector3i a_CursorPosition) const +void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, const Vector3i a_ClickedPosition, const BLOCKTYPE a_ClickedBlockType, const NIBBLETYPE a_ClickedBlockMeta, const eBlockFace a_ClickedBlockFace, const Vector3i a_CursorPosition) const { - if (a_ClickedBlockFace == BLOCK_FACE_NONE) - { - // Clicked in the air, no placement possible - return; - } - - if (!cChunkDef::IsValidHeight(a_ClickedBlockPosition.y)) - { - // The clicked block is outside the world, ignore this call altogether (GH #128): - return; - } - const auto & World = *a_Player.GetWorld(); - BLOCKTYPE ClickedBlockType; - NIBBLETYPE ClickedBlockMeta; - World.GetBlockTypeMeta(a_ClickedBlockPosition, ClickedBlockType, ClickedBlockMeta); // Check if the block ignores build collision (water, grass etc.): - if (cBlockHandler::For(ClickedBlockType).DoesIgnoreBuildCollision(World, a_HeldItem, a_ClickedBlockPosition, ClickedBlockMeta, a_ClickedBlockFace, true)) + if (cBlockHandler::For(a_ClickedBlockType).DoesIgnoreBuildCollision(World, a_HeldItem, a_ClickedPosition, a_ClickedBlockMeta, a_ClickedBlockFace, true)) { // Try to place the block at the clicked position: - if (!CommitPlacement(a_Player, a_HeldItem, a_ClickedBlockPosition, a_ClickedBlockFace, a_CursorPosition)) + if (!CommitPlacement(a_Player, a_HeldItem, a_ClickedPosition, a_ClickedBlockFace, a_CursorPosition)) { // The placement failed, the blocks have already been re-sent, re-send inventory: a_Player.GetInventory().SendEquippedSlot(); @@ -1074,21 +1059,19 @@ void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, c } else { - const auto PlacedPosition = AddFaceDirection(a_ClickedBlockPosition, a_ClickedBlockFace); + BLOCKTYPE PlaceBlock; + NIBBLETYPE PlaceMeta; + const auto PlacePosition = AddFaceDirection(a_ClickedPosition, a_ClickedBlockFace); - if (!cChunkDef::IsValidHeight(PlacedPosition.y)) + if (!cChunkDef::IsValidHeight(PlacePosition.y) || !World.GetBlockTypeMeta(PlacePosition, PlaceBlock, PlaceMeta)) { // The block is being placed outside the world, ignore this packet altogether (GH #128): return; } - NIBBLETYPE PlaceMeta; - BLOCKTYPE PlaceBlock; - World.GetBlockTypeMeta(PlacedPosition, PlaceBlock, PlaceMeta); - // Clicked on side of block, make sure that placement won't be cancelled if there is a slab able to be double slabbed. // No need to do combinability (dblslab) checks, client will do that here. - if (!cBlockHandler::For(PlaceBlock).DoesIgnoreBuildCollision(World, a_HeldItem, PlacedPosition, PlaceMeta, a_ClickedBlockFace, false)) + if (!cBlockHandler::For(PlaceBlock).DoesIgnoreBuildCollision(World, a_HeldItem, PlacePosition, PlaceMeta, a_ClickedBlockFace, false)) { // Tried to place a block into another? // Happens when you place a block aiming at side of block with a torch on it or stem beside it. @@ -1098,7 +1081,7 @@ void cItemHandler::OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, c } // Try to place the block: - if (!CommitPlacement(a_Player, a_HeldItem, PlacedPosition, a_ClickedBlockFace, a_CursorPosition)) + if (!CommitPlacement(a_Player, a_HeldItem, PlacePosition, a_ClickedBlockFace, a_CursorPosition)) { // The placement failed, the blocks have already been re-sent, re-send inventory: a_Player.GetInventory().SendEquippedSlot(); diff --git a/src/Items/ItemHandler.h b/src/Items/ItemHandler.h index da573df9a..8b8c8d2d3 100644 --- a/src/Items/ItemHandler.h +++ b/src/Items/ItemHandler.h @@ -40,13 +40,11 @@ public: /** Called when the player tries to place the item (right mouse button, IsPlaceable() == true). - a_ClickedBlockPos is the (neighbor) block that has been clicked to place this item. - a_ClickedBlockFace is the face of the neighbor that has been clicked to place this item. - a_CursorPos is the position of the player's cursor within a_ClickedBlockFace. - The default handler uses GetBlocksToPlace() and places the returned blocks. - Override if the item needs advanced processing, such as spawning a mob based on the blocks being placed. + a_ClickedPosition is the block that has been clicked to place this item. + a_ClickedBlockFace is the face has been clicked to place this item. + a_CursorPosition is the position of the player's cursor within a_ClickedBlockFace. If the block placement is refused inside this call, it will automatically revert the client-side changes. */ - void OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, Vector3i a_ClickedBlockPosition, eBlockFace a_ClickedBlockFace, Vector3i a_CursorPosition) const; + void OnPlayerPlace(cPlayer & a_Player, const cItem & a_HeldItem, Vector3i a_ClickedPosition, BLOCKTYPE a_ClickedBlockType, NIBBLETYPE a_ClickedBlockMeta, eBlockFace a_ClickedBlockFace, Vector3i a_CursorPosition) const; /** Called when the player tries to use the item (right mouse button). Descendants can return false to abort the usage (default behavior). */ -- cgit v1.2.3