Skip to content

Commit cc24382

Browse files
authored
feat: security hardening from audit (XXE, Newtonsoft type handling, redaction) (#2181)
Security audit found no Critical/High issues; these are defense-in-depth and behavior-change hardenings, safe by default. - xml: force DtdProcessing.Prohibit and clear XmlResolver on every reader settings access to block XXE and entity-expansion attacks, with an obsolete AllowDtdProcessing opt-out for fully trusted sources - newtonsoft: force TypeNameHandling.None when no explicit settings are supplied so an unsafe global JsonConvert.DefaultSettings cannot drive polymorphic deserialization of response bodies; explicit caller settings are still honored - generator: escape U+0085/U+2028/U+2029 in emitted string literals (these break a regular literal, CS1010) and fix the literal length calculation - regex: add a 1s match timeout to the net7+ GeneratedRegex for parity - exceptions: add RefitSettings.ExceptionRedactor to scrub credentials and bodies before an ApiException propagates, and MaxExceptionContentLength to bound the error-body read; document the disclosure surface and make Content/RequestContent settable for redaction - docs: add a V13 breaking-changes section to the README - claude.md: document promoting PublicAPI unshipped to shipped before a PR Tests and coverage: - cover XXE rejection and the DTD opt-out, TypeNameHandling neutralize/honor, generator escaping, content cap, redaction, async error-body reads, and the problem+json validation path; remove dead negative-number handling from the generator's integer writer - perf: cache Roslyn metadata references across generator tests. The harness rebuilt the full framework reference closure (~169 assemblies) per test, so parallel execution multiplied to ~31 GiB across the four TFMs and OOM-killed the CI runner. Materializing each reference once into a process-wide cache drops the net9.0 peak RSS from 8.3 GiB to 1.4 GiB and the four-TFM peak from 30.8 GiB to 4.6 GiB
1 parent db33053 commit cc24382

37 files changed

Lines changed: 816 additions & 118 deletions

CLAUDE.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,3 +159,14 @@ dotnet run --project "tests/Refit.GeneratorTests/Refit.GeneratorTests.csproj" -f
159159
- Keep changes scoped. Do not rewrite unrelated files while touching generator/runtime paths.
160160
- Prefer focused tests that compile or execute the real generated output when changing source generator behavior.
161161

162+
---
163+
164+
## Public API tracking
165+
166+
The runtime projects use the public-API analyzer, so every public/protected member is recorded under `src/Refit/PublicAPI/<tfm>/`.
167+
168+
- New public surface goes in `PublicAPI.Unshipped.txt` for each affected TFM while you iterate.
169+
- **Before opening a PR, move those new entries from `PublicAPI.Unshipped.txt` into the matching `PublicAPI.Shipped.txt` (and reset each unshipped file to just `#nullable enable`).** This repo ships almost immediately after merge, so unshipped API is promoted as part of the change rather than left pending.
170+
- For behavior changes or public API additions, also add a breaking-changes note to the README.
171+
- Shipped entries are assumed to be carried forward; you do not need to call them out separately in the PR.
172+

README.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ services
4040

4141
* [Sponsors](#sponsors)
4242
* [Where does this work?](#where-does-this-work)
43+
* [Breaking changes in V13.x](#breaking-changes-in-v13x)
4344
* [Breaking changes in V12.x](#breaking-changes-in-v12x)
4445
* [Breaking changes in 11.x](#breaking-changes-in-11x)
4546
* [Breaking changes in 6.x](#breaking-changes-in-6x)
@@ -103,6 +104,34 @@ Refit currently supports the following platforms and modern .NET targets:
103104

104105
### SDK Requirements
105106

107+
### V13.x.x
108+
109+
#### Breaking changes in V13.x
110+
111+
Refit 13 hardens the default security posture from a security audit. The new behavior is safe by default; the changes
112+
below only affect code that previously relied on the less-secure defaults.
113+
114+
* **XML responses no longer process DTDs.** `XmlContentSerializer` now forces `DtdProcessing.Prohibit` and clears the
115+
`XmlResolver` on every read, blocking XML External Entity (XXE) and entity-expansion ("billion laughs") attacks. If
116+
you were deserializing trusted XML that depends on a DTD or external entities, that content will now throw. We
117+
**strongly recommend against re-enabling DTD processing**, but if your XML comes from a fully trusted source you can
118+
opt out via the obsolete `XmlReaderWriterSettings.AllowDtdProcessing` flag (marked `[Obsolete]` deliberately, so it
119+
surfaces a compiler warning) - you must also configure `DtdProcessing`/`XmlResolver` yourself on `ReaderSettings`.
120+
Prefer pre-processing untrusted documents instead.
121+
* **Newtonsoft.Json no longer inherits an unsafe global `TypeNameHandling`.** When you do not pass explicit
122+
`JsonSerializerSettings`, `NewtonsoftJsonContentSerializer` now forces `TypeNameHandling.None` even if
123+
`JsonConvert.DefaultSettings` configured a different value, closing a known remote-code-execution gadget vector on
124+
response bodies. If you genuinely need polymorphic (`$type`) deserialization, opt in explicitly by passing your own
125+
`JsonSerializerSettings` (ideally constrained with a `SerializationBinder`).
126+
127+
The release also adds two opt-in, non-breaking knobs on `RefitSettings` for hardening exception handling:
128+
129+
* `RefitSettings.ExceptionRedactor` — a hook invoked before an `ApiException` propagates, so you can scrub the
130+
`Authorization` header, request/response bodies, and `Set-Cookie` before they reach logging or telemetry pipelines
131+
that serialize exceptions.
132+
* `RefitSettings.MaxExceptionContentLength` — caps how many characters of an error response body are read into
133+
`ApiException.Content`, bounding memory use against hostile or oversized error responses. Defaults to unbounded.
134+
106135
### V12.x.x
107136

108137
#### Breaking changes in V12.x

src/InterfaceStubGenerator.Shared/Emitter.Helpers.cs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,73 @@ internal static void AppendEscapedCharacter(StringBuilder builder, char characte
130130
'\r' => builder.Append(@"\r"),
131131
'\t' => builder.Append(@"\t"),
132132
'\v' => builder.Append(@"\v"),
133+
134+
// Line terminators that would break out of a regular C# string literal (CS1010).
135+
'\u0085' => builder.Append(@"\u0085"),
136+
'\u2028' => builder.Append(@"\u2028"),
137+
'\u2029' => builder.Append(@"\u2029"),
133138
_ => builder.Append(character)
134139
};
135140

141+
/// <summary>Gets the escape sequence for a C# string-literal character, or null when none is needed.</summary>
142+
/// <param name="character">The character to escape.</param>
143+
/// <returns>The escape sequence, or <see langword="null"/> when the character is emitted verbatim.</returns>
144+
internal static string? EscapeSequence(char character) =>
145+
character switch
146+
{
147+
'\\' => @"\\",
148+
'"' => "\\\"",
149+
'\0' => @"\0",
150+
'\a' => @"\a",
151+
'\b' => @"\b",
152+
'\f' => @"\f",
153+
'\n' => @"\n",
154+
'\r' => @"\r",
155+
'\t' => @"\t",
156+
'\v' => @"\v",
157+
158+
// Characters C# treats as line terminators inside a regular string literal (CS1010).
159+
'\u0085' => @"\u0085",
160+
'\u2028' => @"\u2028",
161+
'\u2029' => @"\u2029",
162+
_ => null
163+
};
164+
165+
/// <summary>Computes the rendered length of a C# string literal or the <c>null</c> keyword.</summary>
166+
/// <param name="value">The value to quote, or <see langword="null"/>.</param>
167+
/// <returns>The number of characters the rendered expression occupies.</returns>
168+
internal static int LiteralOrNullLength(string? value)
169+
{
170+
if (value is null)
171+
{
172+
return NullLiteral.Length;
173+
}
174+
175+
var length = StringLiteralQuoteLength;
176+
foreach (var character in value)
177+
{
178+
length += EscapeSequence(character) is { Length: var escapeLength } ? escapeLength : 1;
179+
}
180+
181+
return length;
182+
}
183+
184+
/// <summary>Computes the rendered decimal length of a non-negative 32-bit integer.</summary>
185+
/// <param name="value">The non-negative value to render (callers only pass <c>CollectionFormat</c> values).</param>
186+
/// <returns>The number of characters the decimal rendering occupies.</returns>
187+
internal static int Int32Length(int value)
188+
{
189+
var length = 0;
190+
do
191+
{
192+
length++;
193+
value /= DecimalRadix;
194+
}
195+
while (value > 0);
196+
197+
return length;
198+
}
199+
136200
/// <summary>Strips an explicit interface prefix from a method name (e.g. <c>IFoo.Bar</c> becomes <c>Bar</c>).</summary>
137201
/// <param name="name">The method name to normalize.</param>
138202
/// <returns>The method name without any explicit interface prefix.</returns>

src/InterfaceStubGenerator.Shared/Emitter.cs

Lines changed: 10 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -870,61 +870,6 @@ private static string JoinParts(string[] parts, int count, string separator)
870870
/// <returns>The generated indentation.</returns>
871871
private static string Indent(int level) => new(' ', level * CharsPerIndentation);
872872

873-
/// <summary>Gets the two-character escape sequence for a C# string-literal character, or null when none is needed.</summary>
874-
/// <param name="character">The character to escape.</param>
875-
/// <returns>The interned escape sequence, or <see langword="null"/> when the character is emitted verbatim.</returns>
876-
private static string? EscapeSequence(char character) =>
877-
character switch
878-
{
879-
'\\' => "\\\\",
880-
'"' => "\\\"",
881-
'\0' => "\\0",
882-
'\a' => "\\a",
883-
'\b' => "\\b",
884-
'\f' => "\\f",
885-
'\n' => "\\n",
886-
'\r' => "\\r",
887-
'\t' => "\\t",
888-
'\v' => "\\v",
889-
_ => null
890-
};
891-
892-
/// <summary>Computes the rendered length of a C# string literal or the <c>null</c> keyword.</summary>
893-
/// <param name="value">The value to quote, or <see langword="null"/>.</param>
894-
/// <returns>The number of characters the rendered expression occupies.</returns>
895-
private static int LiteralOrNullLength(string? value)
896-
{
897-
if (value is null)
898-
{
899-
return NullLiteral.Length;
900-
}
901-
902-
var length = StringLiteralQuoteLength;
903-
foreach (var character in value)
904-
{
905-
length += EscapeSequence(character) is null ? 1 : StringLiteralQuoteLength;
906-
}
907-
908-
return length;
909-
}
910-
911-
/// <summary>Computes the rendered decimal length of a 32-bit integer.</summary>
912-
/// <param name="value">The value to render.</param>
913-
/// <returns>The number of characters the decimal rendering occupies.</returns>
914-
private static int Int32Length(int value)
915-
{
916-
var length = value < 0 ? 1 : 0;
917-
var magnitude = value < 0 ? -(long)value : value;
918-
do
919-
{
920-
length++;
921-
magnitude /= DecimalRadix;
922-
}
923-
while (magnitude > 0);
924-
925-
return length;
926-
}
927-
928873
#if NETSTANDARD2_0
929874
/// <summary>Writes a C# string literal or the <c>null</c> keyword into a generated string buffer.</summary>
930875
/// <param name="destination">The target character buffer.</param>
@@ -955,26 +900,20 @@ private static void AppendLiteralOrNull(char[] destination, string? value, ref i
955900
destination[position++] = '"';
956901
}
957902

958-
/// <summary>Writes the decimal rendering of a 32-bit integer into a generated string buffer.</summary>
903+
/// <summary>Writes the decimal rendering of a non-negative 32-bit integer into a generated string buffer.</summary>
959904
/// <param name="destination">The target character buffer.</param>
960-
/// <param name="value">The value to render.</param>
905+
/// <param name="value">The non-negative value to render (callers only pass <c>CollectionFormat</c> values).</param>
961906
/// <param name="position">The current write position.</param>
962907
private static void AppendInt32(char[] destination, int value, ref int position)
963908
{
964909
var end = position + Int32Length(value);
965910
var write = end;
966-
var magnitude = value < 0 ? -(long)value : value;
967911
do
968912
{
969-
destination[--write] = (char)('0' + (int)(magnitude % DecimalRadix));
970-
magnitude /= DecimalRadix;
971-
}
972-
while (magnitude > 0);
973-
974-
if (value < 0)
975-
{
976-
destination[position] = '-';
913+
destination[--write] = (char)('0' + (value % DecimalRadix));
914+
value /= DecimalRadix;
977915
}
916+
while (value > 0);
978917

979918
position = end;
980919
}
@@ -1008,26 +947,20 @@ private static void AppendLiteralOrNull(Span<char> destination, string? value, r
1008947
destination[position++] = '"';
1009948
}
1010949

1011-
/// <summary>Writes the decimal rendering of a 32-bit integer into a generated string buffer.</summary>
950+
/// <summary>Writes the decimal rendering of a non-negative 32-bit integer into a generated string buffer.</summary>
1012951
/// <param name="destination">The target character span.</param>
1013-
/// <param name="value">The value to render.</param>
952+
/// <param name="value">The non-negative value to render (callers only pass <c>CollectionFormat</c> values).</param>
1014953
/// <param name="position">The current write position.</param>
1015954
private static void AppendInt32(Span<char> destination, int value, ref int position)
1016955
{
1017956
var end = position + Int32Length(value);
1018957
var write = end;
1019-
var magnitude = value < 0 ? -(long)value : value;
1020958
do
1021959
{
1022-
destination[--write] = (char)('0' + (int)(magnitude % DecimalRadix));
1023-
magnitude /= DecimalRadix;
1024-
}
1025-
while (magnitude > 0);
1026-
1027-
if (value < 0)
1028-
{
1029-
destination[position] = '-';
960+
destination[--write] = (char)('0' + (value % DecimalRadix));
961+
value /= DecimalRadix;
1030962
}
963+
while (value > 0);
1031964

1032965
position = end;
1033966
}

src/Refit.Newtonsoft.Json/NewtonsoftJsonContentSerializer.cs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,26 @@ public sealed class NewtonsoftJsonContentSerializer(
2020
private const int QuotePairLength = 2;
2121

2222
/// <summary>The <see cref="Lazy{T}"/> instance providing the JSON serialization settings to use</summary>
23+
/// <remarks>
24+
/// When the caller does not supply explicit settings, any settings inherited from
25+
/// <see cref="JsonConvert.DefaultSettings"/> have <see cref="TypeNameHandling"/> forced to
26+
/// <see cref="TypeNameHandling.None"/>. A globally configured non-<see cref="TypeNameHandling.None"/>
27+
/// value would otherwise let an attacker-controlled HTTP response body drive polymorphic type
28+
/// resolution (a known remote-code-execution gadget vector). Callers who genuinely need
29+
/// polymorphic deserialization must opt in by passing their own <see cref="JsonSerializerSettings"/>.
30+
/// </remarks>
2331
private readonly Lazy<JsonSerializerSettings> _jsonSerializerSettings =
2432
new(() =>
25-
jsonSerializerSettings
26-
?? JsonConvert.DefaultSettings?.Invoke()
27-
?? new JsonSerializerSettings());
33+
{
34+
if (jsonSerializerSettings is not null)
35+
{
36+
return jsonSerializerSettings;
37+
}
38+
39+
var settings = JsonConvert.DefaultSettings?.Invoke() ?? new JsonSerializerSettings();
40+
settings.TypeNameHandling = TypeNameHandling.None;
41+
return settings;
42+
});
2843

2944
/// <summary>Initializes a new instance of the <see cref="NewtonsoftJsonContentSerializer"/> class.</summary>
3045
public NewtonsoftJsonContentSerializer()

src/Refit.Xml/PublicAPI/net10.0/PublicAPI.Shipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettin
2525
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettings! readerSettings, System.Xml.XmlWriterSettings! writerSettings) -> void
2626
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlWriterSettings! writerSettings) -> void
2727
Refit.XmlContentSerializer.DeserializeFromString<T>(string! content) -> T?
28+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.get -> bool
29+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.set -> void

src/Refit.Xml/PublicAPI/net11.0/PublicAPI.Shipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettin
2525
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettings! readerSettings, System.Xml.XmlWriterSettings! writerSettings) -> void
2626
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlWriterSettings! writerSettings) -> void
2727
Refit.XmlContentSerializer.DeserializeFromString<T>(string! content) -> T?
28+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.get -> bool
29+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.set -> void

src/Refit.Xml/PublicAPI/net462/PublicAPI.Shipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettin
2525
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettings! readerSettings, System.Xml.XmlWriterSettings! writerSettings) -> void
2626
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlWriterSettings! writerSettings) -> void
2727
Refit.XmlContentSerializer.DeserializeFromString<T>(string! content) -> T?
28+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.get -> bool
29+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.set -> void

src/Refit.Xml/PublicAPI/net470/PublicAPI.Shipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettin
2525
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettings! readerSettings, System.Xml.XmlWriterSettings! writerSettings) -> void
2626
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlWriterSettings! writerSettings) -> void
2727
Refit.XmlContentSerializer.DeserializeFromString<T>(string! content) -> T?
28+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.get -> bool
29+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.set -> void

src/Refit.Xml/PublicAPI/net471/PublicAPI.Shipped.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettin
2525
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlReaderSettings! readerSettings, System.Xml.XmlWriterSettings! writerSettings) -> void
2626
Refit.XmlReaderWriterSettings.XmlReaderWriterSettings(System.Xml.XmlWriterSettings! writerSettings) -> void
2727
Refit.XmlContentSerializer.DeserializeFromString<T>(string! content) -> T?
28+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.get -> bool
29+
Refit.XmlReaderWriterSettings.AllowDtdProcessing.set -> void

0 commit comments

Comments
 (0)