Add SortKey validation

Signed-off-by: Robin Dadswell <robin@dadswell.email>

src/NzbDrone.Core.Test/Datastore/SortKeyValidationFixture.cs

@@ -0,0 +1,28 @@
+using FluentAssertions;
+using NUnit.Framework;
+using NzbDrone.Core.Datastore;
+using NzbDrone.Core.Test.Framework;
+
+namespace NzbDrone.Core.Test.Datastore
+{
+    [TestFixture]
+    public class SortKeyValidationFixture : DbTest
+    {
+        [TestCase("amissingcolumn")]
+        [TestCase("amissingtable.id")]
+        [TestCase("table.table.column")]
+        [TestCase("column; DROP TABLE Commands;--")]
+        public void should_return_false_for_invalid_sort_key(string sortKey)
+        {
+            TableMapping.Mapper.IsValidSortKey(sortKey).Should().BeFalse();
+        }
+
+        [TestCase("Id")]
+        [TestCase("id")]
+        [TestCase("commands.id")]
+        public void should_return_true_for_valid_sort_key(string sortKey)
+        {
+            TableMapping.Mapper.IsValidSortKey(sortKey).Should().BeTrue();
+        }
+    }
+}

src/NzbDrone.Core/Datastore/TableMapper.cs

@@ -9,6 +9,8 @@ namespace NzbDrone.Core.Datastore
 {
     public class TableMapper
     {
+        private readonly HashSet<string> _allowedOrderBy = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+
         public TableMapper()
         {
             IgnoreList = new Dictionary<Type, List<PropertyInfo>>();
@@ -28,12 +30,12 @@ namespace NzbDrone.Core.Datastore
 
             if (IgnoreList.TryGetValue(type, out var list))
             {
-                return new ColumnMapper<TEntity>(list, LazyLoadList[type]);
+                return new ColumnMapper<TEntity>(list, LazyLoadList[type], _allowedOrderBy);
             }
 
             IgnoreList[type] = new List<PropertyInfo>();
             LazyLoadList[type] = new List<LazyLoadedProperty>();
-            return new ColumnMapper<TEntity>(IgnoreList[type], LazyLoadList[type]);
+            return new ColumnMapper<TEntity>(IgnoreList[type], LazyLoadList[type], _allowedOrderBy);
         }
 
         public List<PropertyInfo> ExcludeProperties(Type x)
@@ -60,6 +62,35 @@ namespace NzbDrone.Core.Datastore
         {
             return $"SELECT /**select**/ FROM {TableMap[x]} /**join**/ /**innerjoin**/ /**leftjoin**/ /**where**/";
         }
+
+        public bool IsValidSortKey(string sortKey)
+        {
+            string table = null;
+
+            if (sortKey.Contains('.'))
+            {
+                var split = sortKey.Split('.');
+                if (split.Length != 2)
+                {
+                    return false;
+                }
+
+                table = split[0];
+                sortKey = split[1];
+            }
+
+            if (table != null && !TableMap.Values.Contains(table, StringComparer.OrdinalIgnoreCase))
+            {
+                return false;
+            }
+
+            if (!_allowedOrderBy.Contains(sortKey))
+            {
+                return false;
+            }
+
+            return true;
+        }
     }
 
     public class LazyLoadedProperty
@@ -73,17 +104,20 @@ namespace NzbDrone.Core.Datastore
     {
         private readonly List<PropertyInfo> _ignoreList;
         private readonly List<LazyLoadedProperty> _lazyLoadList;
+        private readonly HashSet<string> _allowedOrderBy;
 
-        public ColumnMapper(List<PropertyInfo> ignoreList, List<LazyLoadedProperty> lazyLoadList)
+        public ColumnMapper(List<PropertyInfo> ignoreList, List<LazyLoadedProperty> lazyLoadList, HashSet<string> allowedOrderBy)
         {
             _ignoreList = ignoreList;
             _lazyLoadList = lazyLoadList;
+            _allowedOrderBy = allowedOrderBy;
         }
 
         public ColumnMapper<T> AutoMapPropertiesWhere(Func<PropertyInfo, bool> predicate)
         {
             var properties = typeof(T).GetProperties();
             _ignoreList.AddRange(properties.Where(x => !predicate(x)));
+            _allowedOrderBy.UnionWith(properties.Where(x => predicate(x)).Select(x => x.Name));
 
             return this;
         }

src/Readarr.Http/REST/RestModule.cs

@@ -17,6 +17,20 @@ namespace Readarr.Http.REST
         private const string ROOT_ROUTE = "/";
         private const string ID_ROUTE = @"/(?<id>[\d]{1,10})";
 
+        // See src/Lidarr.Api.V1/Queue/QueueModule.cs
+        private static readonly HashSet<string> VALID_SORT_KEYS = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+        {
+            "timeleft",
+            "estimatedCompletionTime",
+            "protocol",
+            "indexer",
+            "downloadClient",
+            "quality",
+            "status",
+            "title",
+            "progress"
+        };
+
         private readonly HashSet<string> _excludedKeys = new HashSet<string>(StringComparer.InvariantCultureIgnoreCase)
         {
             "page",
@@ -292,7 +306,15 @@ namespace Readarr.Http.REST
 
             if (Request.Query.SortKey != null)
             {
-                pagingResource.SortKey = Request.Query.SortKey.ToString();
+                var sortKey = Request.Query.SortKey.ToString();
+
+                if (!VALID_SORT_KEYS.Contains(sortKey) &&
+                    !TableMapping.Mapper.IsValidSortKey(sortKey))
+                {
+                    throw new BadRequestException($"Invalid sort key {sortKey}");
+                }
+
+                pagingResource.SortKey = sortKey;
 
                 // For backwards compatibility with v2
                 if (Request.Query.SortDir != null)