From 6c5452570e7274654762751fb242f619e9db72d8 Mon Sep 17 00:00:00 2001 From: Regalis Date: Tue, 16 Aug 2016 17:56:33 +0300 Subject: [PATCH] More sanity checks to make sure clients aren't doing something they're not allowed to: - AICharacter, hull, structure and submarine updates from clients are ignored - character updates from anyone else than the client controlling the character are ignored - players can't pick/drop items from anyone else's inventory (unless the target is unconscious/stunned/cuffed) - server has authority over reactor temperature --- Subsurface/Source/Characters/AICharacter.cs | 25 ++-- Subsurface/Source/Characters/Character.cs | 131 +++++++++--------- Subsurface/Source/Items/CharacterInventory.cs | 14 +- .../Items/Components/Machines/Reactor.cs | 13 +- Subsurface/Source/Items/Inventory.cs | 14 +- Subsurface/Source/Items/Item.cs | 30 ++-- Subsurface/Source/Map/Entity.cs | 17 ++- Subsurface/Source/Map/Hull.cs | 12 +- Subsurface/Source/Map/Structure.cs | 8 +- Subsurface/Source/Map/Submarine.cs | 12 +- Subsurface/Source/Networking/NetworkEvent.cs | 5 +- 11 files changed, 175 insertions(+), 106 deletions(-) diff --git a/Subsurface/Source/Characters/AICharacter.cs b/Subsurface/Source/Characters/AICharacter.cs index 651e952d3..4e9ecb0bb 100644 --- a/Subsurface/Source/Characters/AICharacter.cs +++ b/Subsurface/Source/Characters/AICharacter.cs @@ -138,19 +138,22 @@ namespace Barotrauma } } - public override void ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) + public override bool ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) { data = null; Enabled = true; + //server doesn't accept AICharacter updates from the clients + if (GameMain.Server != null) return false; + switch (type) { case NetworkEventType.KillCharacter: + Kill(CauseOfDeath.Damage, true); - return; + break; case NetworkEventType.InventoryUpdate: - base.ReadNetworkData(type, message, sendingTime, out data); - return; + return base.ReadNetworkData(type, message, sendingTime, out data); case NetworkEventType.ImportantEntityUpdate: Vector2 limbPos = AnimController.RefLimb.SimPosition; @@ -168,7 +171,7 @@ namespace Barotrauma #if DEBUG DebugConsole.ThrowError("Failed to read AICharacter update message", e); #endif - return; + return false; } if (AnimController.RefLimb.body != null) @@ -199,7 +202,7 @@ namespace Barotrauma DebugConsole.ThrowError("Failed to read AICharacter update message", e); #endif - return; + return false; } AnimController.StunTimer = newStunTimer; @@ -208,9 +211,9 @@ namespace Barotrauma Bleeding = newBleeding; aiController.ReadNetworkData(message); - return; + break; case NetworkEventType.EntityUpdate: - if (sendingTime <= LastNetworkUpdate) return; + if (sendingTime <= LastNetworkUpdate) return false; Vector2 targetMovement = Vector2.Zero, pos = Vector2.Zero; bool targetDir = false,inSub = false; @@ -231,7 +234,7 @@ namespace Barotrauma #if DEBUG DebugConsole.ThrowError("Failed to read AICharacter update message", e); #endif - return; + return false; } AnimController.TargetDir = (targetDir) ? Direction.Right : Direction.Left; @@ -251,8 +254,10 @@ namespace Barotrauma } LastNetworkUpdate = sendingTime; - return; + break; } + + return true; } diff --git a/Subsurface/Source/Characters/Character.cs b/Subsurface/Source/Characters/Character.cs index f5f893224..166707df7 100644 --- a/Subsurface/Source/Characters/Character.cs +++ b/Subsurface/Source/Characters/Character.cs @@ -777,6 +777,28 @@ namespace Barotrauma } } + public bool CanAccessInventory(Inventory inventory) + { + if (inventory.Owner is Character && inventory.Owner != this) + { + var owner = (Character)inventory.Owner; + + return owner.isDead || owner.IsUnconscious || owner.Stun > 0.0f || owner.LockHands; + } + + return true; + } + + public bool CanAccessItem(Item item) + { + if (item.ParentInventory != null) + { + if (!CanAccessInventory(item.ParentInventory)) return false; + } + + return true; + } + private Item FindClosestItem(Vector2 mouseSimPos, out float distance) { distance = 0.0f; @@ -1337,46 +1359,7 @@ namespace Barotrauma } Kill(CauseOfDeath.Pressure, isNetworkMessage); } - - //private IEnumerable DeathAnim(Camera cam) - //{ - // if (controlled != this) yield return CoroutineStatus.Success; - - // Character.controlled = null; - - // float dimDuration = 8.0f; - // float timer = 0.0f; - - // Color prevAmbientLight = GameMain.LightManager.AmbientLight; - // Color darkLight = new Color(0.2f, 0.2f, 0.2f, 1.0f); - - // while (timer < dimDuration && Character.controlled == null) - // { - // timer += CoroutineManager.UnscaledDeltaTime; - - // if (cam != null) cam.OffsetAmount = 0.0f; - - // cam.TargetPos = WorldPosition; - - // GameMain.LightManager.AmbientLight = Color.Lerp(prevAmbientLight, darkLight, timer / dimDuration); - - // yield return CoroutineStatus.Running; - // } - - // float lerpLightBack = 0.0f; - // while (lerpLightBack < 1.0f) - // { - // lerpLightBack = Math.Min(lerpLightBack + CoroutineManager.UnscaledDeltaTime*5.0f, 1.0f); - - // GameMain.LightManager.AmbientLight = Color.Lerp(darkLight, prevAmbientLight, lerpLightBack); - // yield return CoroutineStatus.Running; - // } - - // cam.TargetPos = Vector2.Zero; - - // yield return CoroutineStatus.Success; - //} - + public void Kill(CauseOfDeath causeOfDeath, bool isNetworkMessage = false) { if (isDead) return; @@ -1604,14 +1587,27 @@ namespace Barotrauma } } - public override void ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) + public override bool ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) { Enabled = true; data = null; + if (GameMain.Server != null && type != NetworkEventType.InventoryUpdate) + { + Client sender = GameMain.Server.ConnectedClients.Find(c => c.Connection == message.SenderConnection); + if (sender == null || sender.Character != this) + { +#if DEBUG + DebugConsole.ThrowError("Received a character update message from someone else than the client controlling the Character!"); +#endif + return false; + } + } + switch (type) { case NetworkEventType.PickItem: + ushort itemId = message.ReadUInt16(); bool pickHit = message.ReadBoolean(); @@ -1626,6 +1622,8 @@ namespace Barotrauma Item pickedItem = FindEntityByID(itemId) as Item; if (pickedItem != null) { + if (!CanAccessItem(pickedItem)) return false; + if (pickedItem == selectedConstruction) { GameServer.Log(Name + " deselected " + pickedItem.Name, Color.Orange); @@ -1638,7 +1636,7 @@ namespace Barotrauma selectedConstruction = isSelected ? pickedItem : null; } - return; + break; case NetworkEventType.SelectCharacter: bool performingCPR = message.ReadBoolean(); @@ -1648,11 +1646,11 @@ namespace Barotrauma if (characterId==0) { DeselectCharacter(false); - return; + return true; } Character character = FindEntityByID(characterId) as Character; - if (character == null || !character.IsHumanoid) return; + if (character == null || !character.IsHumanoid) return true; SelectCharacter(character, false); if (performingCPR) @@ -1668,16 +1666,9 @@ namespace Barotrauma { AnimController.Anim = AnimController.Animation.None; } - - return; - case NetworkEventType.KillCharacter: - if (GameMain.Server != null) - { - Client sender = GameMain.Server.ConnectedClients.Find(c => c.Connection == message.SenderConnection); - if (sender == null || sender.Character != this) - throw new Exception("Received a KillCharacter message from someone else than the client controlling the Character!"); - } + break; + case NetworkEventType.KillCharacter: CauseOfDeath causeOfDeath = CauseOfDeath.Damage; try { @@ -1699,24 +1690,30 @@ namespace Barotrauma { Kill(causeOfDeath, true); } - return; + break; case NetworkEventType.InventoryUpdate: - if (inventory == null) return; + if (inventory == null) return false; + inventory.ReadNetworkData(NetworkEventType.InventoryUpdate, message, sendingTime); - return; + return true; case NetworkEventType.ApplyStatusEffect: ushort id = message.ReadUInt16(); data = id; var item = FindEntityByID(id) as Item; - if (item == null) return; + if (item == null) return false; item.ApplyStatusEffects(ActionType.OnUse, 1.0f, this); break; case NetworkEventType.ImportantEntityUpdate: + if (GameMain.Server != null) + { + return false; + } + health = message.ReadRangedSingle(minHealth, 100.0f, 8); bool allOk = message.ReadBoolean(); @@ -1725,16 +1722,16 @@ namespace Barotrauma bleeding = 0.0f; Oxygen = 100.0f; AnimController.StunTimer = 0.0f; - return; + return true; } float newStunTimer = message.ReadRangedSingle(0.0f, 60.0f, 8); StartStun(newStunTimer, true); Oxygen = message.ReadRangedSingle(-100.0f,100.0f, 8); - Bleeding = message.ReadRangedSingle(0.0f, 5.0f, 8); + Bleeding = message.ReadRangedSingle(0.0f, 5.0f, 8); - return; + break; case NetworkEventType.EntityUpdate: Vector2 relativeCursorPos = Vector2.Zero; @@ -1763,10 +1760,10 @@ namespace Barotrauma #if DEBUG DebugConsole.ThrowError("Error in Character.ReadNetworkData: " + e.Message); #endif - return; + return false; } - if (GameMain.Server != null && (isDead || IsUnconscious)) return; + if (GameMain.Server != null && (isDead || IsUnconscious)) return false; keys[(int)InputType.Use].Held = actionKeyState; keys[(int)InputType.Use].SetState(false, actionKeyState); @@ -1774,7 +1771,7 @@ namespace Barotrauma keys[(int)InputType.Aim].Held = secondaryKeyState; keys[(int)InputType.Aim].SetState(false, secondaryKeyState); - if (sendingTime <= LastNetworkUpdate) return; + if (sendingTime <= LastNetworkUpdate) return false; keys[(int)InputType.Left].Held = leftKeyState; keys[(int)InputType.Right].Held = rightKeyState; @@ -1813,7 +1810,7 @@ namespace Barotrauma #if DEBUG DebugConsole.ThrowError("Failed to read networkevent for "+this.ToString()); #endif - return; + return false; } bool inSub = message.ReadBoolean(); @@ -1863,13 +1860,15 @@ namespace Barotrauma LastNetworkUpdate = sendingTime; - return; + break; default: #if DEBUG DebugConsole.ThrowError("Character " + this + " tried to read a networkevent of the wrong type: " + type); #endif - return; + return false; } + + return true; } public override void Remove() diff --git a/Subsurface/Source/Items/CharacterInventory.cs b/Subsurface/Source/Items/CharacterInventory.cs index 0bfcad17f..717b28ea8 100644 --- a/Subsurface/Source/Items/CharacterInventory.cs +++ b/Subsurface/Source/Items/CharacterInventory.cs @@ -526,8 +526,7 @@ namespace Barotrauma if (Items[i] != null) { droppedItems.Add(Items[i]); - Items[i].Drop(character, false); - + Items[i].Drop(character, false); } } else @@ -538,6 +537,17 @@ namespace Barotrauma //item already in the right slot, no need to do anything if (Items[i] == item) continue; + //if the item is in the inventory of some other character and said character isn't dead/unconscious, + //don't let this character pick it up + if (GameMain.Server != null) + { + var owner = item.ParentInventory == null ? null : item.ParentInventory.Owner as Character; + if (owner != null && owner != character) + { + if (!character.CanAccessItem(item)) return; + } + } + //some other item already in the slot -> drop it if (Items[i] != null) Items[i].Drop(character, false); diff --git a/Subsurface/Source/Items/Components/Machines/Reactor.cs b/Subsurface/Source/Items/Components/Machines/Reactor.cs index 20a304a6e..82f409c2a 100644 --- a/Subsurface/Source/Items/Components/Machines/Reactor.cs +++ b/Subsurface/Source/Items/Components/Machines/Reactor.cs @@ -539,7 +539,10 @@ namespace Barotrauma.Items.Components public override bool FillNetworkData(NetworkEventType type, NetBuffer message) { message.Write(autoTemp); - message.WriteRangedSingle(temperature, 0.0f, 10000.0f, 16); + if (GameMain.Server != null) + { + message.WriteRangedSingle(temperature, 0.0f, 10000.0f, 16); + } message.WriteRangedSingle(shutDownTemp, 0.0f, 10000.0f, 8); message.WriteRangedSingle(coolingRate, 0.0f, 100.0f, 8); @@ -553,13 +556,16 @@ namespace Barotrauma.Items.Components if (sendingTime < lastUpdate) return; bool newAutoTemp; - float newTemperature, newShutDownTemp; + float newTemperature = Temperature, newShutDownTemp; float newCoolingRate, newFissionRate; try { newAutoTemp = message.ReadBoolean(); - newTemperature = message.ReadRangedSingle(0.0f, 10000.0f, 16); + if (GameMain.Server == null) + { + newTemperature = message.ReadRangedSingle(0.0f, 10000.0f, 16); + } newShutDownTemp = message.ReadRangedSingle(0.0f, 10000.0f, 8); newShutDownTemp = MathUtils.RoundTowardsClosest(newShutDownTemp, 100.0f); @@ -585,7 +591,6 @@ namespace Barotrauma.Items.Components lastUpdate = sendingTime; if (GameMain.Server == null) return; - var sender = GameMain.Server.ConnectedClients.Find(c => c.Connection == message.SenderConnection); if (sender != null) { diff --git a/Subsurface/Source/Items/Inventory.cs b/Subsurface/Source/Items/Inventory.cs index ea63d0428..b8be3982a 100644 --- a/Subsurface/Source/Items/Inventory.cs +++ b/Subsurface/Source/Items/Inventory.cs @@ -403,11 +403,13 @@ namespace Barotrauma //List newItemIDs = new List(); //List droppedItems = new List(); List prevItems = new List(Items); + + Client sender = GameMain.Server == null ? null : GameMain.Server.ConnectedClients.Find(c => c.Connection == message.SenderConnection); for (int i = 0; i < capacity; i++) { ushort itemId = message.ReadUInt16(); - if (itemId==0) + if (itemId == 0) { if (Items[i] != null) Items[i].Drop(); } @@ -416,6 +418,15 @@ namespace Barotrauma var item = Entity.FindEntityByID(itemId) as Item; if (item == null) continue; + //if the item is in the inventory of some other character and said character isn't dead/unconscious, + //don't let this character pick it up + if (GameMain.Server != null) + { + if (sender == null || sender.Character == null) return; + + if (!sender.Character.CanAccessItem(item)) continue; + } + TryPutItem(item, i, true, false); } } @@ -423,7 +434,6 @@ namespace Barotrauma lastUpdate = sendingTime; if (GameMain.Server == null) return; - var sender = GameMain.Server.ConnectedClients.Find(c => c.Connection == message.SenderConnection); if (sender != null && sender.Character != null) { foreach (Item item in Items) diff --git a/Subsurface/Source/Items/Item.cs b/Subsurface/Source/Items/Item.cs index 336f99082..149b12a48 100644 --- a/Subsurface/Source/Items/Item.cs +++ b/Subsurface/Source/Items/Item.cs @@ -1217,7 +1217,7 @@ namespace Barotrauma return closest; } - + public bool IsInsideTrigger(Vector2 worldPosition) { foreach (Rectangle trigger in prefab.Triggers) @@ -1716,7 +1716,7 @@ namespace Barotrauma return true; } - public override void ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) + public override bool ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) { data = null; @@ -1725,6 +1725,15 @@ namespace Barotrauma switch (type) { case NetworkEventType.DropItem: + if (GameMain.Server != null) + { + Client sender = GameMain.Server.ConnectedClients.Find(c => c.Connection == message.SenderConnection); + if (sender == null || sender.Character == null || !sender.Character.CanAccessItem(this)) + { + return false; + } + } + Drop(null, false); if (body != null) { @@ -1733,22 +1742,23 @@ namespace Barotrauma } break; case NetworkEventType.PhysicsBodyPosition: + //clients don't have authority over item positions + if (GameMain.Server != null) return false; if (body != null) body.ReadNetworkData(message, sendingTime); FindHull(); break; case NetworkEventType.ItemFixed: - byte requirementIndex = message.ReadByte(); data = requirementIndex; - if (requirementIndex >= FixRequirements.Count) return; + if (requirementIndex >= FixRequirements.Count) return false; FixRequirements[requirementIndex].Fixed = true; break; case NetworkEventType.InventoryUpdate: var itemContainers = GetComponents(); - if (itemContainers == null || !itemContainers.Any()) return; + if (itemContainers == null || !itemContainers.Any()) return false; int containerCount = message.ReadRangedInteger(1, ItemContainer.MaxInventoryCount); for (int i = 0; i < containerCount;i++ ) @@ -1763,7 +1773,7 @@ namespace Barotrauma data = componentIndex; - if (componentIndex < 0 || componentIndex > components.Count - 1) return; + if (componentIndex < 0 || componentIndex > components.Count - 1) return false; components[componentIndex].NetworkUpdateSent = true; components[componentIndex].ReadNetworkData(type, message, sendingTime); @@ -1787,12 +1797,12 @@ namespace Barotrauma } catch { - return; + return false; } var allProperties = GetProperties(); ObjectProperty property = allProperties.Find(op => op.Name == propertyName); - if (property == null) return; + if (property == null) return false; try { @@ -1815,11 +1825,13 @@ namespace Barotrauma catch { - return; + return false; } break; } + + return true; } public override void Remove() diff --git a/Subsurface/Source/Map/Entity.cs b/Subsurface/Source/Map/Entity.cs index 01abcb297..d77fd77de 100644 --- a/Subsurface/Source/Map/Entity.cs +++ b/Subsurface/Source/Map/Entity.cs @@ -93,13 +93,28 @@ namespace Barotrauma dictionary.Add(id, this); } + /// + /// Writes the state of the entity into the message + /// + /// some data that was saved when the networkevent was created + /// false if something went wrong when filling the message, true if the msg is ready to be sent public virtual bool FillNetworkData(NetworkEventType type, NetBuffer message, object data) { return false; } - public virtual void ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) + + /// + /// Updates the state of the entity based on the data in the message + /// + + /// + /// + /// false if the message is not valid (client trying to change something they're not authorized to, corrupt data, etc) and should be ignored + public virtual bool ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) { data = null; + + return false; } /// diff --git a/Subsurface/Source/Map/Hull.cs b/Subsurface/Source/Map/Hull.cs index 37044b886..ce6770a5a 100644 --- a/Subsurface/Source/Map/Hull.cs +++ b/Subsurface/Source/Map/Hull.cs @@ -831,11 +831,13 @@ namespace Barotrauma return true; } - public override void ReadNetworkData(Networking.NetworkEventType type, Lidgren.Network.NetIncomingMessage message, float sendingTime, out object data) + public override bool ReadNetworkData(Networking.NetworkEventType type, Lidgren.Network.NetIncomingMessage message, float sendingTime, out object data) { data = null; - if (sendingTime < lastNetworkUpdate) return; + if (GameMain.Server != null) return false; + + if (sendingTime < lastNetworkUpdate) return false; float newVolume = this.volume; float newOxygen = this.OxygenPercentage; @@ -851,7 +853,7 @@ namespace Barotrauma catch (Exception e) { DebugConsole.Log("Failed to read network message for Hull {" + ID + "}! " + e.Message); - return; + return false; } Volume = newVolume; @@ -901,8 +903,6 @@ namespace Barotrauma } } - - var toBeRemoved = fireSources.FindAll(fs => !newFireSources.Contains(fs)); for (int i = toBeRemoved.Count - 1; i >= 0; i--) { @@ -911,6 +911,8 @@ namespace Barotrauma fireSources = newFireSources; lastNetworkUpdate = sendingTime; + + return true; } diff --git a/Subsurface/Source/Map/Structure.cs b/Subsurface/Source/Map/Structure.cs index 49f9d5219..3c359006f 100644 --- a/Subsurface/Source/Map/Structure.cs +++ b/Subsurface/Source/Map/Structure.cs @@ -793,11 +793,13 @@ namespace Barotrauma return true; } - public override void ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) + public override bool ReadNetworkData(NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) { data = null; - if (sendingTime < lastUpdate) return; + if (GameMain.Server != null) return false; + + if (sendingTime < lastUpdate) return false; // int sectionCount = message.ReadByte(); @@ -812,6 +814,8 @@ namespace Barotrauma } lastUpdate = sendingTime; + + return true; } } diff --git a/Subsurface/Source/Map/Submarine.cs b/Subsurface/Source/Map/Submarine.cs index 720fc5dee..587269b26 100644 --- a/Subsurface/Source/Map/Submarine.cs +++ b/Subsurface/Source/Map/Submarine.cs @@ -550,14 +550,16 @@ namespace Barotrauma return true; } - public override void ReadNetworkData(Networking.NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) + public override bool ReadNetworkData(Networking.NetworkEventType type, NetIncomingMessage message, float sendingTime, out object data) { data = null; + if (GameMain.Server != null) return false; + Vector2 newTargetPosition, newSpeed; try { - if (sendingTime <= lastNetworkUpdate) return; + if (sendingTime <= lastNetworkUpdate) return false; newTargetPosition = new Vector2(message.ReadFloat(), message.ReadFloat()); newSpeed = new Vector2(message.ReadFloat(), message.ReadFloat()); @@ -568,10 +570,10 @@ namespace Barotrauma #if DEBUG DebugConsole.ThrowError("invalid network message", e); #endif - return; + return false; } - if (!newSpeed.IsValid() || !newTargetPosition.IsValid()) return; + if (!newSpeed.IsValid() || !newTargetPosition.IsValid()) return false; //newTargetPosition = newTargetPosition + newSpeed * (float)(NetTime.Now - sendingTime); @@ -579,6 +581,8 @@ namespace Barotrauma subBody.Velocity = newSpeed; lastNetworkUpdate = sendingTime; + + return true; } diff --git a/Subsurface/Source/Networking/NetworkEvent.cs b/Subsurface/Source/Networking/NetworkEvent.cs index 3f440aff8..67fdc6352 100644 --- a/Subsurface/Source/Networking/NetworkEvent.cs +++ b/Subsurface/Source/Networking/NetworkEvent.cs @@ -227,7 +227,10 @@ namespace Barotrauma.Networking try { - e.ReadNetworkData(eventType, message, sendingTime, out data); + if (!e.ReadNetworkData(eventType, message, sendingTime, out data)) + { + resend = false; + } } catch (Exception exception) {