From 93774a0234522185101c8bcb58f7d5ac301cdc01 Mon Sep 17 00:00:00 2001
From: Ryan Brandenburg <rybrande@microsoft.com>
Date: Wed, 11 Jan 2017 15:10:24 -0800
Subject: [PATCH] Mitigate MediaType overflow

---
 .../Formatters/MediaType.cs                   |  8 ++++-
 .../Properties/Resources.Designer.cs          | 16 ++++++++++
 .../Resources.resx                            |  4 +++
 .../Formatters/MediaTypeTest.cs               | 31 +++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs
index 4835e11606..7d37dcbd9b 100644
--- a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs
+++ b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/MediaType.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Globalization;
 using System.Text;
+using Microsoft.AspNetCore.Mvc.Core;
 using Microsoft.AspNetCore.Mvc.Formatters.Internal;
 using Microsoft.Extensions.Primitives;
 
@@ -54,11 +55,16 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
                 throw new ArgumentOutOfRangeException(nameof(offset));
             }
 
-            if (length != null && offset + length > mediaType.Length)
+            if (length < 0 || length > mediaType.Length)
             {
                 throw new ArgumentOutOfRangeException(nameof(length));
             }
 
+            if (offset > mediaType.Length - length)
+            {
+                throw new ArgumentException(Resources.FormatArgument_InvalidOffsetLength(nameof(offset), nameof(length)));
+            }
+
             _parameterParser = default(MediaTypeParameterParser);
 
             StringSegment type;
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs
index c9f9d5b058..f1f70e2c2d 100644
--- a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs
+++ b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs
@@ -1386,6 +1386,22 @@ namespace Microsoft.AspNetCore.Mvc.Core
             return string.Format(CultureInfo.CurrentCulture, GetString("MiddlewareFilterConfigurationProvider_CreateConfigureDelegate_CannotCreateType"), p0, p1);
         }
 
+        /// <summary>
+        /// '{0}' and '{1}' are out of bounds for the string.
+        /// </summary>
+        internal static string Argument_InvalidOffsetLength
+        {
+            get { return GetString("Argument_InvalidOffsetLength"); }
+        }
+
+        /// <summary>
+        /// '{0}' and '{1}' are out of bounds for the string.
+        /// </summary>
+        internal static string FormatArgument_InvalidOffsetLength(object p0, object p1)
+        {
+            return string.Format(CultureInfo.CurrentCulture, GetString("Argument_InvalidOffsetLength"), p0, p1);
+        }
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx
index f6236e6e64..37f5c7b895 100644
--- a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx
+++ b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx
@@ -387,4 +387,8 @@
     <value>Unable to create an instance of type '{0}'. The type specified in {1} must not be abstract and must have a parameterless constructor.</value>
     <comment>0 is the type to configure. 1 is the name of the parameter, configurationType.</comment>
   </data>
+  <data name="Argument_InvalidOffsetLength" xml:space="preserve">
+    <value>'{0}' and '{1}' are out of bounds for the string.</value>
+    <comment>'{0}' and '{1}' are the parameters which combine to be out of bounds.</comment>
+  </data>
 </root>
\ No newline at end of file
diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs
index f79075f2e8..b5bd23dacd 100644
--- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs
+++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/MediaTypeTest.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System;
 using System.Text;
 using Microsoft.Extensions.Primitives;
 using Xunit;
@@ -59,6 +60,36 @@ namespace Microsoft.AspNetCore.Mvc.Formatters
             Assert.Equal(new StringSegment("utf-8"), result.GetParameter("charset"));
         }
 
+        [Fact]
+        public void Constructor_NullMediaType_Throws()
+        {
+            // Arrange, Act and Assert
+            Assert.Throws<ArgumentNullException>("mediaType", () => new MediaType(null, 0, 2));
+        }
+
+        [Theory]
+        [InlineData(-1)]
+        [InlineData(7)]
+        public void Constructor_NegativeOffset_Throws(int offset)
+        {
+            // Arrange, Act and Assert
+            Assert.Throws<ArgumentOutOfRangeException>("offset", () => new MediaType("media", offset, 5));
+        }
+
+        [Fact]
+        public void Constructor_NegativeLength_Throws()
+        {
+            // Arrange, Act and Assert
+            Assert.Throws<ArgumentOutOfRangeException>("length", () => new MediaType("media", 0, -1));
+        }
+
+        [Fact]
+        public void Constructor_OffsetOrLengthOutOfBounds_Throws()
+        {
+            // Arrange, Act and Assert
+            Assert.Throws<ArgumentException>(() => new MediaType("lengthof9", 5, 5));
+        }
+
         [Theory]
         [MemberData(nameof(MediaTypesWithParameters))]
         public void ReplaceEncoding_ReturnsExpectedMediaType(string mediaType)