Fix CodeQL SM02211 alert - SerializationBinder

libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsStorage.cs

@@ -13,6 +13,7 @@
 using Azure.Storage.Blobs;
 using Azure.Storage.Blobs.Models;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 
 namespace Microsoft.Bot.Builder.Azure.Blobs
 {
@@ -29,6 +30,13 @@ namespace Microsoft.Bot.Builder.Azure.Blobs
     /// </remarks>
     public class BlobsStorage : IStorage
     {
+        private static readonly AllowedTypesSerializationBinder _allowedTypesBinder = new AllowedTypesSerializationBinder(
+            new List<Type>
+            {
+                typeof(IStoreItem),
+                typeof(Dictionary<string, object>)
+            });
+
         // If a JsonSerializer is not provided during construction, this will be the default JsonSerializer.
         private readonly JsonSerializer _jsonSerializer;
         private readonly BlobContainerClient _containerClient;
@@ -44,6 +52,7 @@ public class BlobsStorage : IStorage
         /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.None.</para>
         /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
         /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
         /// </param>
         public BlobsStorage(string dataConnectionString, string containerName, JsonSerializer jsonSerializer = null)
             : this(dataConnectionString, containerName, default, jsonSerializer)
@@ -60,6 +69,7 @@ public BlobsStorage(string dataConnectionString, string containerName, JsonSeria
         /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.None.</para>
         /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
         /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
         /// </param>
         public BlobsStorage(string dataConnectionString, string containerName, StorageTransferOptions storageTransferOptions, JsonSerializer jsonSerializer = null)
         {
@@ -77,7 +87,8 @@ public BlobsStorage(string dataConnectionString, string containerName, StorageTr
 
             _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(new JsonSerializerSettings
                                                 {
-                                                    TypeNameHandling = TypeNameHandling.All,
+                                                    TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling]
+                                                    SerializationBinder = _allowedTypesBinder,
                                                     MaxDepth = null,
                                                 });
 
@@ -95,14 +106,16 @@ public BlobsStorage(string dataConnectionString, string containerName, StorageTr
         /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.None.</para>
         /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
         /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
         /// </param>
         internal BlobsStorage(BlobContainerClient containerClient, JsonSerializer jsonSerializer = null)
         {
             _containerClient = containerClient;
 
             _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(new JsonSerializerSettings
             {
-                TypeNameHandling = TypeNameHandling.All,
+                TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling]
+                SerializationBinder = _allowedTypesBinder,
                 MaxDepth = null,
             });
 
@@ -214,7 +227,19 @@ internal BlobsStorage(BlobContainerClient containerClient, JsonSerializer jsonSe
                 {
                     using var memoryStream = new MemoryStream();
                     using var streamWriter = new StreamWriter(memoryStream);
-                    _jsonSerializer.Serialize(streamWriter, newValue);
+                    using var jsonWriter = new JsonTextWriter(streamWriter);
+
+                    var json = JToken.FromObject(newValue, _jsonSerializer);
+                    if (json.Type == JTokenType.Object || json.Type == JTokenType.Array)
+                    {
+                        (_jsonSerializer.SerializationBinder as AllowedTypesSerializationBinder)?.CleanupTypes((JContainer)json);
+                        await json.WriteToAsync(jsonWriter).ConfigureAwait(false);
+                    }
+                    else
+                    {
+                        _jsonSerializer.Serialize(streamWriter, newValue);
+                    }
+
                     await streamWriter.FlushAsync().ConfigureAwait(false);
                     memoryStream.Seek(0, SeekOrigin.Begin);
 

libraries/Microsoft.Bot.Builder.Azure.Blobs/BlobsTranscriptStore.cs

@@ -80,7 +80,6 @@ public BlobsTranscriptStore(string dataConnectionString, string containerName, S
                                                 {
                                                     NullValueHandling = NullValueHandling.Ignore,
                                                     Formatting = Formatting.Indented,
-                                                    TypeNameHandling = TypeNameHandling.None,
                                                     MaxDepth = null,
                                                 });
 
@@ -115,7 +114,6 @@ internal BlobsTranscriptStore(BlobContainerClient containerClient, JsonSerialize
 
             _jsonSerializer = jsonSerializer ?? JsonSerializer.Create(new JsonSerializerSettings
             {
-                TypeNameHandling = TypeNameHandling.All,
                 MaxDepth = null,
             });
         }

libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs

@@ -14,6 +14,7 @@
 using Microsoft.WindowsAzure.Storage.Core;
 using Microsoft.WindowsAzure.Storage.RetryPolicies;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 
 namespace Microsoft.Bot.Builder.Azure
 {
@@ -33,8 +34,13 @@ public class AzureBlobStorage : IStorage
     {
         private static readonly JsonSerializer JsonSerializer = JsonSerializer.Create(new JsonSerializerSettings
         {
-            // we use All so that we get typed roundtrip out of storage, but we don't use validation because we don't know what types are valid
-            TypeNameHandling = TypeNameHandling.All,
+            TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling]
+            SerializationBinder = new AllowedTypesSerializationBinder(
+                new List<Type>
+                {
+                    typeof(IStoreItem),
+                    typeof(Dictionary<string, object>)
+                }),
             MaxDepth = null,
         });
 
@@ -54,6 +60,7 @@ public class AzureBlobStorage : IStorage
         /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.All.</para>
         /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
         /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
         /// </param>
         public AzureBlobStorage(CloudStorageAccount storageAccount, string containerName, JsonSerializer jsonSerializer)
         {
@@ -94,8 +101,14 @@ public AzureBlobStorage(CloudStorageAccount storageAccount, string containerName
         /// <param name="storageAccount">Azure CloudStorageAccount instance.</param>
         /// <param name="containerName">Name of the Blob container where entities will be stored.</param>
         /// <param name="blobClient">Custom implementation of CloudBlobClient.</param>
-        internal AzureBlobStorage(CloudStorageAccount storageAccount, string containerName, CloudBlobClient blobClient)
-            : this(storageAccount, containerName, JsonSerializer)
+        /// <param name="jsonSerializer">If passing in a custom JsonSerializer, we recommend the following settings:
+        /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.All.</para>
+        /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
+        /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
+        /// </param>
+        internal AzureBlobStorage(CloudStorageAccount storageAccount, string containerName, CloudBlobClient blobClient, JsonSerializer jsonSerializer = default)
+            : this(storageAccount, containerName, jsonSerializer ?? JsonSerializer)
         {
             _blobClient = blobClient;
         }
@@ -211,8 +224,18 @@ internal AzureBlobStorage(CloudStorageAccount storageAccount, string containerNa
                 {
                     using (var memoryStream = new MultiBufferMemoryStream(blobReference.ServiceClient.BufferManager))
                     using (var streamWriter = new StreamWriter(memoryStream))
+                    using (var jsonWriter = new JsonTextWriter(streamWriter))
                     {
-                        _jsonSerializer.Serialize(streamWriter, newValue);
+                        var json = JToken.FromObject(newValue, _jsonSerializer);
+                        if (json.Type == JTokenType.Object || json.Type == JTokenType.Array)
+                        {
+                            (_jsonSerializer.SerializationBinder as AllowedTypesSerializationBinder)?.CleanupTypes((JContainer)json);
+                            await json.WriteToAsync(jsonWriter).ConfigureAwait(false);
+                        }
+                        else
+                        {
+                            _jsonSerializer.Serialize(streamWriter, newValue);
+                        }
 
                         await streamWriter.FlushAsync().ConfigureAwait(false);
 

libraries/Microsoft.Bot.Builder.Azure/CosmosDbPartitionedStorage.cs

@@ -21,7 +21,18 @@ namespace Microsoft.Bot.Builder.Azure
     public class CosmosDbPartitionedStorage : IStorage, IDisposable
     {
         private const int MaxDepthAllowed = 127;
-        private readonly JsonSerializer _jsonSerializer = JsonSerializer.Create(new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.All, MaxDepth = null });
+
+        private readonly JsonSerializer _jsonSerializer = JsonSerializer.Create(new JsonSerializerSettings
+        {
+            TypeNameHandling = TypeNameHandling.Objects, // lgtm [cs/unsafe-type-name-handling]
+            SerializationBinder = new AllowedTypesSerializationBinder(
+                new List<Type>
+                {
+                    typeof(IStoreItem),
+                    typeof(Dictionary<string, object>)
+                }),
+            MaxDepth = null
+        });
 
         private Container _container;
         private readonly CosmosDbPartitionedStorageOptions _cosmosDbStorageOptions;
@@ -91,6 +102,7 @@ public CosmosDbPartitionedStorage(CosmosDbPartitionedStorageOptions cosmosDbStor
         /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.All.</para>
         /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
         /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
         /// </param>
         public CosmosDbPartitionedStorage(CosmosDbPartitionedStorageOptions cosmosDbStorageOptions, JsonSerializer jsonSerializer)
             : this(cosmosDbStorageOptions)
@@ -105,10 +117,20 @@ public CosmosDbPartitionedStorage(CosmosDbPartitionedStorageOptions cosmosDbStor
         /// </summary>
         /// <param name="client">The custom implementation of CosmosClient.</param>
         /// <param name="cosmosDbStorageOptions">Cosmos DB partitioned storage configuration options.</param>
-        internal CosmosDbPartitionedStorage(CosmosClient client, CosmosDbPartitionedStorageOptions cosmosDbStorageOptions)
+        /// <param name="jsonSerializer">If passing in a custom JsonSerializer, we recommend the following settings:
+        /// <para>jsonSerializer.TypeNameHandling = TypeNameHandling.All.</para>
+        /// <para>jsonSerializer.NullValueHandling = NullValueHandling.Include.</para>
+        /// <para>jsonSerializer.ContractResolver = new DefaultContractResolver().</para>
+        /// <para>jsonSerializer.SerializationBinder = new AllowedTypesSerializationBinder().</para>
+        /// </param>
+        internal CosmosDbPartitionedStorage(CosmosClient client, CosmosDbPartitionedStorageOptions cosmosDbStorageOptions, JsonSerializer jsonSerializer = default)
             : this(cosmosDbStorageOptions)
         {
             _client = client;
+            if (jsonSerializer != null)
+            {
+                _jsonSerializer = jsonSerializer;
+            }
         }
 
         /// <summary>
@@ -205,6 +227,7 @@ internal CosmosDbPartitionedStorage(CosmosClient client, CosmosDbPartitionedStor
             foreach (var change in changes)
             {
                 var json = JObject.FromObject(change.Value, _jsonSerializer);
+                (_jsonSerializer.SerializationBinder as AllowedTypesSerializationBinder)?.CleanupTypes(json);
 
                 // Remove etag from JSON object that was copied from IStoreItem.
                 // The ETag information is updated as an _etag attribute in the document metadata.
@@ -381,7 +404,7 @@ private async Task InitializeAsync()
                         try
                         {
                             _container = _client.GetContainer(_cosmosDbStorageOptions.DatabaseId, _cosmosDbStorageOptions.ContainerId);
-                            
+
                             // This will throw if the container does not exist. 
                             var readContainer = await _container.ReadContainerAsync().ConfigureAwait(false);