Skip to content

Commit

Permalink
Enforce a maximum value when parsing unix timestamps (#981)
Browse files Browse the repository at this point in the history
* UnixTimestampConverter should now obey a maximum value

This change prevents an issue where the converter would be unable to
handle obscenely large timestamp values - which are actually quite
common on Discord.

OptionalConverter had to be rewritten to support checking whether or not
an InnerConverter returned an Optional. The perf impacts from this
_shouldn't_ be too bad, as types without a custom parser (which should
be the majority of Optionals in the lib) will bypass the type-check.

* optimizations on OptionalConverter
  • Loading branch information
foxbot committed Mar 18, 2018
1 parent 02c6507 commit bfaa6fc
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
12 changes: 10 additions & 2 deletions src/Discord.Net.Rest/Net/Converters/OptionalConverter.cs
@@ -1,4 +1,4 @@
using Newtonsoft.Json;
using Newtonsoft.Json;
using System;

namespace Discord.Net.Converters
Expand All @@ -19,10 +19,18 @@ public OptionalConverter(JsonConverter innerConverter)
public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
T obj;
// custom converters need to be able to safely fail; move this check in here to prevent wasteful casting when parsing primitives
if (_innerConverter != null)
obj = (T)_innerConverter.ReadJson(reader, typeof(T), null, serializer);
{
object o = _innerConverter.ReadJson(reader, typeof(T), null, serializer);
if (o is Optional<T>)
return o;

obj = (T)o;
}
else
obj = serializer.Deserialize<T>(reader);

return new Optional<T>(obj);
}

Expand Down
17 changes: 11 additions & 6 deletions src/Discord.Net.Rest/Net/Converters/UnixTimestampConverter.cs
@@ -1,4 +1,4 @@
using System;
using System;
using Newtonsoft.Json;

namespace Discord.Net.Converters
Expand All @@ -11,18 +11,23 @@ public class UnixTimestampConverter : JsonConverter
public override bool CanRead => true;
public override bool CanWrite => true;

// 1e13 unix ms = year 2286
// necessary to prevent discord.js from sending values in the e15 and overflowing a DTO
private const long MaxSaneMs = 1_000_000_000_000_0;

public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
{
// Discord doesn't validate if timestamps contain decimals or not
if (reader.Value is double d)
// Discord doesn't validate if timestamps contain decimals or not, and they also don't validate if timestamps are reasonably sized
if (reader.Value is double d && d < MaxSaneMs)
return new DateTimeOffset(1970, 1, 1, 0, 0, 0, 0, TimeSpan.Zero).AddMilliseconds(d);
long offset = (long)reader.Value;
return new DateTimeOffset(1970, 1, 1, 0, 0, 0, 0, TimeSpan.Zero).AddMilliseconds(offset);
else if (reader.Value is long l && l < MaxSaneMs)
return new DateTimeOffset(1970, 1, 1, 0, 0, 0, 0, TimeSpan.Zero).AddMilliseconds(l);
return Optional<DateTimeOffset>.Unspecified;
}

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
{
throw new NotImplementedException();
}
}
}
}

0 comments on commit bfaa6fc

Please sign in to comment.