From b1ef7ffd059cdf8b429e6c031486b51fd1947780 Mon Sep 17 00:00:00 2001 From: Regalis Date: Wed, 5 Apr 2017 23:09:00 +0300 Subject: [PATCH] Replaced excessively paranoid filename validation regex in FileReceiver with Path.GetInvalidFileNameChars, server is notified if a client cancels a file transfer for whatever reason --- .../Source/Networking/FileTransfer/FileReceiver.cs | 12 ++++++++---- Subsurface/Source/Networking/GameClient.cs | 7 ++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Subsurface/Source/Networking/FileTransfer/FileReceiver.cs b/Subsurface/Source/Networking/FileTransfer/FileReceiver.cs index 2708c43a2..0afc107a3 100644 --- a/Subsurface/Source/Networking/FileTransfer/FileReceiver.cs +++ b/Subsurface/Source/Networking/FileTransfer/FileReceiver.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; -using System.Text; using System.Text.RegularExpressions; -using System.Threading.Tasks; using System.Xml; namespace Barotrauma.Networking @@ -183,6 +181,7 @@ namespace Barotrauma.Networking var existingTransfer = activeTransfers.Find(t => t.SequenceChannel == inc.SequenceChannel); if (existingTransfer != null) { + GameMain.Client.CancelFileTransfer(inc.SequenceChannel); DebugConsole.ThrowError("File transfer error: file transfer initiated on a sequence channel that's already in use"); return; } @@ -195,7 +194,8 @@ namespace Barotrauma.Networking string errorMsg; if (!ValidateInitialData(fileType, fileName, fileSize, out errorMsg)) { - DebugConsole.ThrowError("File transfer failed ("+errorMsg+")"); + GameMain.Client.CancelFileTransfer(inc.SequenceChannel); + DebugConsole.ThrowError("File transfer failed (" + errorMsg + ")"); return; } @@ -219,12 +219,14 @@ namespace Barotrauma.Networking var activeTransfer = activeTransfers.Find(t => t.Connection == inc.SenderConnection && t.SequenceChannel == inc.SequenceChannel); if (activeTransfer == null) { + GameMain.Client.CancelFileTransfer(inc.SequenceChannel); DebugConsole.ThrowError("File transfer error: received data without a transfer initiation message"); return; } if (activeTransfer.Received + (ulong)(inc.LengthBytes-inc.PositionInBytes) > activeTransfer.FileSize) { + GameMain.Client.CancelFileTransfer(inc.SequenceChannel); DebugConsole.ThrowError("File transfer error: Received more data than expected"); activeTransfer.Status = FileTransferStatus.Error; StopTransfer(activeTransfer); @@ -237,6 +239,7 @@ namespace Barotrauma.Networking } catch (Exception e) { + GameMain.Client.CancelFileTransfer(inc.SequenceChannel); DebugConsole.ThrowError("File transfer error: "+e.Message); activeTransfer.Status = FileTransferStatus.Error; StopTransfer(activeTransfer, true); @@ -291,7 +294,8 @@ namespace Barotrauma.Networking return false; } - if (!Regex.Match(fileName, @"^[\w\- ]+[\w\-. ]*$").Success) + if (string.IsNullOrEmpty(fileName) || + fileName.IndexOfAny(Path.GetInvalidFileNameChars()) > -1) { errorMessage = "Illegal characters in file name ''" + fileName + "''"; return false; diff --git a/Subsurface/Source/Networking/GameClient.cs b/Subsurface/Source/Networking/GameClient.cs index bfeac6fb6..dcbda50f0 100644 --- a/Subsurface/Source/Networking/GameClient.cs +++ b/Subsurface/Source/Networking/GameClient.cs @@ -999,11 +999,16 @@ namespace Barotrauma.Networking } public void CancelFileTransfer(FileReceiver.FileTransferIn transfer) + { + CancelFileTransfer(transfer); + } + + public void CancelFileTransfer(int sequenceChannel) { NetOutgoingMessage msg = client.CreateMessage(); msg.Write((byte)ClientPacketHeader.FILE_REQUEST); msg.Write((byte)FileTransferMessageType.Cancel); - msg.Write((byte)transfer.SequenceChannel); + msg.Write((byte)sequenceChannel); client.SendMessage(msg, NetDeliveryMethod.ReliableUnordered); }