Skip to content

Commit

Permalink
FileCache.SerializableCacheItemPolicy fix
Browse files Browse the repository at this point in the history
Removed BinaryFormatter usage from SerializableCacheItemPolicy.

SerializableCacheItemPolicy now (de)serializes via functions:
  - Serialize()
  - Deserialize()
These use BinaryWriter/BinaryReader instead of the security risk
BinaryFormatter.

SerializableCacheItemPolicy now use a magic header number like the
cached files themselves do, in order to differentiate BinaryFormatter vs
new policies.
  • Loading branch information
cole-brown committed Jul 28, 2021
1 parent d738c50 commit 82ff59b
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 12 deletions.
35 changes: 24 additions & 11 deletions src/FileCache/FileCacheManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ public abstract class FileCacheManager
{
// Magic version for new sysfiles: 3.3.0 packed into a long.
protected const ulong CACHE_VERSION = ( 3 << 16
+ 3 << 8
+ 0 << 0);
+ 3 << 8
+ 0 << 0);

public string CacheDir { get; set; }
public string CacheSubFolder { get; set; }
Expand Down Expand Up @@ -127,8 +127,21 @@ public virtual FileCachePayload ReadFile(FileCache.PayloadMode mode, string key,
}
try
{
// TODO: In part of the merge it looked like the policy was force serialized with LocalCacheBinder(), is this intended?
payload.Policy = Deserialize(policyPath) as SerializableCacheItemPolicy;
if (File.Exists(policyPath))
{
using (FileStream stream = GetStream(policyPath, FileMode.Open, FileAccess.Read, FileShare.Read))
{
using (BinaryReader reader = new BinaryReader(stream))
{
// TODO: In part of the merge it looked like the policy was force serialized with LocalCacheBinder(), is this intended?
payload.Policy = SerializableCacheItemPolicy.Deserialize(reader, stream.Length);
}
}
}
else
{
payload.Policy = new SerializableCacheItemPolicy();
}
}
catch
{
Expand Down Expand Up @@ -268,13 +281,13 @@ public virtual long WriteFile(FileCache.PayloadMode mode, string key, FileCacheP
//write the cache policy
using (FileStream stream = GetStream(cachedPolicy, FileMode.Create, FileAccess.Write, FileShare.None))
{
BinaryFormatter formatter = new BinaryFormatter();
formatter.Serialize(stream, data.Policy);

// adjust cache size
cacheSizeDelta += new FileInfo(cachedPolicy).Length;
using (BinaryWriter writer = new BinaryWriter(stream))
{
data.Policy.Serialize(writer);

stream.Close();
// adjust cache size
cacheSizeDelta += new FileInfo(cachedPolicy).Length;
}
}

return cacheSizeDelta;
Expand Down Expand Up @@ -555,7 +568,7 @@ public override Type BindToType(string assemblyName, string typeName)

// Get the type using the typeName and assemblyName
typeToDeserialize = Type.GetType(String.Format("{0}, {1}",
typeName, assemblyName));
typeName, assemblyName));

return typeToDeserialize;
}
Expand Down
74 changes: 73 additions & 1 deletion src/FileCache/SerializableCacheItemPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,19 @@ FileCache is distributed under the Apache License 2.0.
Consult "LICENSE.txt" included in this package for the Apache License 2.0.
*/

using System.IO;


namespace System.Runtime.Caching
{
[Serializable]
public class SerializableCacheItemPolicy
{
// Magic version for new policies: 3.3.0 packed into a long.
protected const ulong CACHE_VERSION = ( 3 << 16
+ 3 << 8
+ 0 << 0);

public DateTimeOffset AbsoluteExpiration { get; set; }

private TimeSpan _slidingExpiration;
Expand All @@ -24,7 +32,7 @@ public TimeSpan SlidingExpiration
set
{
_slidingExpiration = value;
if (_slidingExpiration > new TimeSpan())
if (_slidingExpiration > TimeSpan.Zero)
{
AbsoluteExpiration = DateTimeOffset.Now.Add(_slidingExpiration);
}
Expand All @@ -45,5 +53,69 @@ public SerializableCacheItemPolicy()
/// The cache key that this particular policy refers to
/// </summary>
public string Key { get; set; }

/// <summary>
/// Serialize this policy to the supplied BinaryWriter.
///
/// Older policies use the "[Serializable]" attribute and BinaryFormatter, which is a security risk:
/// https://docs.microsoft.com/nl-nl/dotnet/standard/serialization/binaryformatter-security-guide#preferred-alternatives
///
/// The newer caches have a 'magic' header we'll look for and serialize their fields manually.
/// </summary>
public void Serialize(BinaryWriter writer)
{
writer.Write(CACHE_VERSION);

writer.Write(AbsoluteExpiration.Date.ToBinary());
writer.Write(AbsoluteExpiration.Offset.TotalMilliseconds);

writer.Write(SlidingExpiration.TotalMilliseconds);

writer.Write(Key);
}

/// <summary>
/// Deserialize a policy from the supplied BinaryReader.
///
/// Older policies use the "[Serializable]" attribute and BinaryFormatter, which is a security risk:
/// https://docs.microsoft.com/nl-nl/dotnet/standard/serialization/binaryformatter-security-guide#preferred-alternatives
///
/// The newer caches have a 'magic' header we'll look for and deserialize their fields manually.
/// If the 'magic' header isn't found, this returns an empty policy.
/// </summary>
public static SerializableCacheItemPolicy Deserialize(BinaryReader reader, long streamLength)
{
// Can't even check for the magic version number; return empty policy.
if (streamLength < sizeof(ulong))
return new SerializableCacheItemPolicy();

try
{
var version = reader.ReadUInt64();
if (version != CACHE_VERSION)
// Just return an empty policy if we read an invalid one.
// This is likely the older "BinaryFormatter"-serialized policy.
return new SerializableCacheItemPolicy();

return new SerializableCacheItemPolicy {
AbsoluteExpiration = new DateTimeOffset(DateTime.FromBinary(reader.ReadInt64()),
TimeSpan.FromMilliseconds(reader.ReadDouble())),
SlidingExpiration = TimeSpan.FromMilliseconds(reader.ReadDouble()),
Key = reader.ReadString(),
};
}
catch (Exception error)
{
if (error is EndOfStreamException
|| error is IOException
|| error is ObjectDisposedException)
{
// Just return an empty policy if we failed to read.
return new SerializableCacheItemPolicy();
}
// Didn't expect this error type; rethrow it.
throw;
}
}
}
}

0 comments on commit 82ff59b

Please sign in to comment.