From 7a62468ffbb90008e08b30471bfa94b2d4c58726 Mon Sep 17 00:00:00 2001
From: Badre BSAILA <54767641+pedrobsaila@users.noreply.github.com>
Date: Wed, 1 Dec 2021 21:16:59 +0100
Subject: [PATCH] throw exception when creating an entry name that already
 exists in ZipArchive (#60973)

* throw exception when creating an entry name that already exists in ZipArchive

* secure code at the top of CreateEntry call

* specify entry name in exception message

* fix exception message, make AddEntry retrocompatible, and fix test issue with globalization

* restore the EmptyEntryTest code as before
---
 .../src/Resources/Strings.resx                |  3 ++
 .../src/System/IO/Compression/ZipArchive.cs   | 12 +++----
 .../tests/ZipArchive/zip_CreateTests.cs       | 17 ++++++++++
 .../tests/ZipArchive/zip_UpdateTests.cs       | 32 +++++++++++++++----
 4 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/src/libraries/System.IO.Compression/src/Resources/Strings.resx b/src/libraries/System.IO.Compression/src/Resources/Strings.resx
index 286ff6ab7c723..cac801aa8fcc4 100644
--- a/src/libraries/System.IO.Compression/src/Resources/Strings.resx
+++ b/src/libraries/System.IO.Compression/src/Resources/Strings.resx
@@ -308,4 +308,7 @@
   <data name="Zip64EOCDNotWhereExpected" xml:space="preserve">
     <value>Zip 64 End of Central Directory Record not where indicated.</value>
   </data>
+  <data name="EntryNameAlreadyExists" xml:space="preserve">
+    <value>An entry named '{0}' already exists in the archive.</value>
+  </data>
 </root>
diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
index 5fbd00c47006c..58b65209caf0e 100644
--- a/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
+++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchive.cs
@@ -388,6 +388,11 @@ private ZipArchiveEntry DoCreateEntry(string entryName, CompressionLevel? compre
             if (_mode == ZipArchiveMode.Read)
                 throw new NotSupportedException(SR.CreateInReadMode);
 
+            if (_entriesDictionary.ContainsKey(entryName))
+            {
+                throw new InvalidOperationException(string.Format(SR.EntryNameAlreadyExists, entryName));
+            }
+
             ThrowIfDisposed();
 
 
@@ -421,12 +426,7 @@ internal void AcquireArchiveStream(ZipArchiveEntry entry)
         private void AddEntry(ZipArchiveEntry entry)
         {
             _entries.Add(entry);
-
-            string entryName = entry.FullName;
-            if (!_entriesDictionary.ContainsKey(entryName))
-            {
-                _entriesDictionary.Add(entryName, entry);
-            }
+            _entriesDictionary.TryAdd(entry.FullName, entry);
         }
 
         [Conditional("DEBUG")]
diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.cs
index b5075cd8ba594..f0c36d5967626 100644
--- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.cs
+++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_CreateTests.cs
@@ -178,6 +178,23 @@ public static void CreateNormal_VerifyDataDescriptor()
             AssertDataDescriptor(memoryStream, false);
         }
 
+        [Fact]
+        public static void CreateNormal_With2SameEntries_ThrowException()
+        {
+            using var memoryStream = new MemoryStream();
+            // We need an non-seekable stream so the data descriptor bit is turned on when saving
+            var wrappedStream = new WrappedStream(memoryStream);
+
+            // Creation will go through the path that sets the data descriptor bit when the stream is unseekable
+            using (var archive = new ZipArchive(wrappedStream, ZipArchiveMode.Create))
+            {
+                string entryName = "duplicate.txt";
+                CreateEntry(archive, entryName, "xxx");
+                AssertExtensions.ThrowsContains<InvalidOperationException>(() => CreateEntry(archive, entryName, "yyy"),
+                    entryName);
+            }
+        }
+
         private static string ReadStringFromSpan(Span<byte> input)
         {
             return Text.Encoding.UTF8.GetString(input.ToArray());
diff --git a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs
index af97a9d3ab960..3cb9934fb3a7c 100644
--- a/src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs
+++ b/src/libraries/System.IO.Compression/tests/ZipArchive/zip_UpdateTests.cs
@@ -92,20 +92,38 @@ public static void EmptyEntryTest(ZipArchiveMode mode)
             baseline = baseline.Clone();
             using (ZipArchive archive = new ZipArchive(baseline, mode))
             {
-                AddEntry(archive, "data1.txt", data1, lastWrite);
+                if (mode == ZipArchiveMode.Create)
+                {
+                    AddEntry(archive, "data1.txt", data1, lastWrite);
 
-                ZipArchiveEntry e = archive.CreateEntry("empty.txt");
-                e.LastWriteTime = lastWrite;
-                using (Stream s = e.Open()) { }
+                    ZipArchiveEntry e = archive.CreateEntry("empty.txt");
+                    e.LastWriteTime = lastWrite;
+                    using (Stream s = e.Open()) { }
+                }
+                else
+                {
+                    Assert.Throws<InvalidOperationException>(() => AddEntry(archive, "data1.txt", data1, lastWrite));
+
+                    Assert.Throws<InvalidOperationException>(() => archive.CreateEntry("empty.txt"));
+                }
             }
 
             test = test.Clone();
             using (ZipArchive archive = new ZipArchive(test, mode))
             {
-                AddEntry(archive, "data1.txt", data1, lastWrite);
+                if (mode == ZipArchiveMode.Create)
+                {
+                    AddEntry(archive, "data1.txt", data1, lastWrite);
 
-                ZipArchiveEntry e = archive.CreateEntry("empty.txt");
-                e.LastWriteTime = lastWrite;
+                    ZipArchiveEntry e = archive.CreateEntry("empty.txt");
+                    e.LastWriteTime = lastWrite;
+                }
+                else
+                {
+                    Assert.Throws<InvalidOperationException>(() => AddEntry(archive, "data1.txt", data1, lastWrite));
+
+                    Assert.Throws<InvalidOperationException>(() => archive.CreateEntry("empty.txt"));
+                }
             }
             //compare
             Assert.True(ArraysEqual(baseline.ToArray(), test.ToArray()), "Arrays didn't match after update");