Sanitize player position and speed server-side (#12396)

diff --git a/src/player.h b/src/player.h
index d769acdad8067d9a0a25e64c62b5f237156830c0..cc13570103873feb176673551b3f3f322cdcfd2f 100644 (file)
--- a/src/player.h
+++ b/src/player.h
@@ -134,13 +134,14 @@ class Player
                        std::vector<CollisionInfo> *collision_info)
        {}
 
-       const v3f &getSpeed() const
+       v3f getSpeed() const
        {
                return m_speed;
        }
 
-       void setSpeed(const v3f &speed)
+       void setSpeed(v3f speed)
        {
+               clampToF1000(speed);
                m_speed = speed;
        }
 

diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp
index d076d578342906a900f9f87874412531daca2d1f..27759ba9d73b5d3b49ae19e8d4dd8da922f456bf 100644 (file)
--- a/src/server/player_sao.cpp
+++ b/src/server/player_sao.cpp
@@ -319,8 +319,14 @@ std::string PlayerSAO::generateUpdatePhysicsOverrideCommand() const
        return os.str();
 }
 
-void PlayerSAO::setBasePosition(const v3f &position)
+void PlayerSAO::setBasePosition(v3f position)
 {
+       // It's not entirely clear which parts of the network protocol still use
+       // v3f1000, but the script API enforces its bound on all float vectors
+       // (maybe it shouldn't?). For that reason we need to make sure the position
+       // isn't ever set to values that fail this restriction.
+       clampToF1000(position);
+
        if (m_player && position != m_base_position)
                m_player->setDirty(true);
 
@@ -344,7 +350,7 @@ void PlayerSAO::setPos(const v3f &pos)
 
        setBasePosition(pos);
        // Movement caused by this command is always valid
-       m_last_good_position = pos;
+       m_last_good_position = getBasePosition();
        m_move_pool.empty();
        m_time_from_last_teleport = 0.0;
        m_env->getGameDef()->SendMovePlayer(m_peer_id);
@@ -357,7 +363,7 @@ void PlayerSAO::moveTo(v3f pos, bool continuous)
 
        setBasePosition(pos);
        // Movement caused by this command is always valid
-       m_last_good_position = pos;
+       m_last_good_position = getBasePosition();
        m_move_pool.empty();
        m_time_from_last_teleport = 0.0;
        m_env->getGameDef()->SendMovePlayer(m_peer_id);

diff --git a/src/server/player_sao.h b/src/server/player_sao.h
index 1067801e7bdfcafe36b7d0b374ee42f3dbb7e246..b84bf1e823ed49d834740bce58b5d3e8dc4b0c0b 100644 (file)
--- a/src/server/player_sao.h
+++ b/src/server/player_sao.h
@@ -87,7 +87,7 @@ class PlayerSAO : public UnitSAO
        std::string getClientInitializationData(u16 protocol_version) override;
        void getStaticData(std::string *result) const override;
        void step(float dtime, bool send_recommended) override;
-       void setBasePosition(const v3f &position);
+       void setBasePosition(v3f position);
        void setPos(const v3f &pos) override;
        void moveTo(v3f pos, bool continuous) override;
        void setPlayerYaw(const float yaw);

diff --git a/src/util/serialize.h b/src/util/serialize.h
index 15bdd050daa6426521640cd798cb2b831c9669d3..2203fff0ceb3e22abbaec76cdbf7fa3041c62ea2 100644 (file)
--- a/src/util/serialize.h
+++ b/src/util/serialize.h
@@ -439,6 +439,18 @@ MAKE_STREAM_WRITE_FXN(video::SColor, ARGB8, 4);
 //// More serialization stuff
 ////
 
+inline void clampToF1000(float &v)
+{
+       v = core::clamp(v, F1000_MIN, F1000_MAX);
+}
+
+inline void clampToF1000(v3f &v)
+{
+       clampToF1000(v.X);
+       clampToF1000(v.Y);
+       clampToF1000(v.Z);
+}
+
 // Creates a string with the length as the first two bytes
 std::string serializeString16(const std::string &plain);