Skip to content

Commit

Permalink
Merge pull request #280 from SixLabors/js/billion-lols
Browse files Browse the repository at this point in the history
Prevent crash from Billion Laughs attack.
  • Loading branch information
JimBobSquarePants authored Jun 27, 2022
2 parents 034a440 + 7448d62 commit 1b80769
Show file tree
Hide file tree
Showing 32 changed files with 172 additions and 83 deletions.
1 change: 0 additions & 1 deletion src/SixLabors.Fonts/GlyphMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ internal FontRectangle GetBoundingBox(Vector2 origin, float scaledPointSize)

internal void RenderDecorationsTo(IGlyphRenderer renderer, Vector2 location, float scaledPPEM)
{
// TODO: Move to base class
(Vector2 Start, Vector2 End, float Thickness) GetEnds(float thickness, float position)
{
Vector2 scale = new Vector2(scaledPPEM) / this.ScaleFactor * MirrorScale;
Expand Down
6 changes: 3 additions & 3 deletions src/SixLabors.Fonts/GlyphPositioningCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public bool TryAdd(Font font, GlyphSubstitutionCollection collection)
/// </summary>
/// <param name="fontMetrics">The font metrics.</param>
/// <param name="index">The zero-based index of the element.</param>
public void UpdatePosition(FontMetrics fontMetrics, ushort index)
public void UpdatePosition(FontMetrics fontMetrics, int index)
{
GlyphShapingData data = this.GetGlyphShapingData(index);
bool isDirtyXY = data.Bounds.IsDirtyXY;
Expand Down Expand Up @@ -338,7 +338,7 @@ public void UpdatePosition(FontMetrics fontMetrics, ushort index)
/// <param name="glyphId">The id of the glyph to offset.</param>
/// <param name="dx">The delta x-advance.</param>
/// <param name="dy">The delta y-advance.</param>
public void Advance(FontMetrics fontMetrics, ushort index, ushort glyphId, short dx, short dy)
public void Advance(FontMetrics fontMetrics, int index, ushort glyphId, short dx, short dy)
{
foreach (GlyphMetrics m in this.glyphs[index].Metrics)
{
Expand All @@ -355,7 +355,7 @@ public void Advance(FontMetrics fontMetrics, ushort index, ushort glyphId, short
/// <param name="fontMetrics">The font face with metrics.</param>
/// <param name="index">The zero-based index of the elements to position.</param>
/// <returns><see langword="true"/> if the element should be processed; otherwise, <see langword="false"/>.</returns>
public bool ShouldProcess(FontMetrics fontMetrics, ushort index)
public bool ShouldProcess(FontMetrics fontMetrics, int index)
=> this.glyphs[index].Metrics[0].FontMetrics == fontMetrics;

private class GlyphPositioningData
Expand Down
9 changes: 6 additions & 3 deletions src/SixLabors.Fonts/GlyphSubstitutionCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,12 @@ public void Replace(int index, ReadOnlySpan<ushort> glyphIds)
glyphIds = glyphIds.Slice(1);
for (int i = 0; i < glyphIds.Length; i++)
{
GlyphShapingData data = new(current, false);
data.GlyphId = glyphIds[i];
data.LigatureComponent = i + 1;
GlyphShapingData data = new(current, false)
{
GlyphId = glyphIds[i],
LigatureComponent = i + 1
};

this.glyphs.Insert(++index, new(pair.Offset, data));
}
}
Expand Down
14 changes: 11 additions & 3 deletions src/SixLabors.Fonts/StreamFontMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ internal override void UpdatePositions(GlyphPositioningCollection collection)

bool kerned = false;
KerningMode kerningMode = collection.TextOptions.KerningMode;

gpos?.TryUpdatePositions(this, collection, out kerned);

if (!kerned && kerningMode != KerningMode.None)
Expand All @@ -263,11 +264,18 @@ internal override void UpdatePositions(GlyphPositioningCollection collection)
? this.trueTypeFontTables!.Kern
: this.compactFontTables!.Kern;

if (kern != null)
if (kern?.Count > 0)
{
for (ushort index = 1; index < collection.Count; index++)
// Set max constraints to prevent OutOfMemoryException or infinite loops from attacks.
int maxCount = AdvancedTypographicUtils.GetMaxAllowableShapingCollectionCount(collection.Count);
for (int index = 1; index < collection.Count; index++)
{
kern.UpdatePositions(this, collection, (ushort)(index - 1), index);
if (index >= maxCount)
{
break;
}

kern.UpdatePositions(this, collection, index - 1, index);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,21 @@ namespace SixLabors.Fonts.Tables.AdvancedTypographic
{
internal static class AdvancedTypographicUtils
{
/// <summary>
/// The maximum length of a context. Taken from HarfBuzz - hb-ot-layout-common.hh
/// </summary>
// The following properties are used to prevent overflows caused
// by maliciously crafted fonts.
// Based on HarfBuzz hb-buffer.hh
public const int MaxContextLength = 64;
private const int MaxLengthFactor = 64;
private const int MaxLengthMinimum = 16384;
private const int MaxOperationsFactor = 1024;
private const int MaxOperationsMinimum = 16384;
private const int MaxShapingCharsLength = 0x3FFFFFFF; // Half int max.

public static int GetMaxAllowableShapingCollectionCount(int length)
=> (int)Math.Min(Math.Max((long)length * MaxLengthFactor, MaxLengthMinimum), MaxShapingCharsLength);

public static int GetMaxAllowableShapingOperationsCount(int length)
=> (int)Math.Min(Math.Max((long)length * MaxOperationsFactor, MaxOperationsMinimum), MaxShapingCharsLength);

public static bool ApplyLookupList(
FontMetrics fontMetrics,
Expand All @@ -22,7 +33,7 @@ public static bool ApplyLookupList(
LookupFlags lookupFlags,
SequenceLookupRecord[] records,
GlyphSubstitutionCollection collection,
ushort index,
int index,
int count)
{
bool hasChanged = false;
Expand Down Expand Up @@ -56,7 +67,7 @@ public static bool ApplyLookupList(
LookupFlags lookupFlags,
SequenceLookupRecord[] records,
GlyphPositioningCollection collection,
ushort index,
int index,
int count)
{
bool hasChanged = false;
Expand Down Expand Up @@ -189,7 +200,7 @@ public static bool CheckAllCoverages(
FontMetrics fontMetrics,
LookupFlags lookupFlags,
IGlyphShapingCollection collection,
ushort index,
int index,
int count,
CoverageTable[] input,
CoverageTable[] backtrack,
Expand Down Expand Up @@ -224,7 +235,7 @@ public static bool CheckAllCoverages(
public static void ApplyAnchor(
FontMetrics fontMetrics,
GlyphPositioningCollection collection,
ushort index,
int index,
AnchorTable baseAnchor,
MarkRecord markRecord,
int baseGlyphIndex)
Expand All @@ -242,7 +253,7 @@ public static void ApplyAnchor(

public static void ApplyPosition(
GlyphPositioningCollection collection,
ushort index,
int index,
ValueRecord record)
{
GlyphShapingData current = collection.GetGlyphShapingData(index);
Expand Down Expand Up @@ -302,8 +313,8 @@ private static bool Match<T>(
Func<T, GlyphShapingData, bool> condition,
Span<int> matches)
{
ushort position = iterator.Index;
ushort offset = iterator.Increment(increment);
int position = iterator.Index;
int offset = iterator.Increment(increment);
IGlyphShapingCollection collection = iterator.Collection;

int i = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
foreach (LookupSubTable subTable in this.LookupSubTables)
Expand Down Expand Up @@ -172,7 +172,7 @@ public abstract bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
ushort glyphId = collection[index];
Expand Down Expand Up @@ -141,7 +141,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
ushort glyphId = collection[index];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
if (count <= 1)
Expand Down Expand Up @@ -122,7 +122,7 @@ public override bool TryUpdatePosition(
AdvancedTypographicUtils.ApplyPosition(collection, index, record1);

ValueRecord record2 = pairValueRecord.ValueRecord2;
AdvancedTypographicUtils.ApplyPosition(collection, (ushort)(index + 1), record2);
AdvancedTypographicUtils.ApplyPosition(collection, index + 1, record2);

return true;
}
Expand Down Expand Up @@ -257,7 +257,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
if (count <= 1)
Expand Down Expand Up @@ -290,7 +290,7 @@ public override bool TryUpdatePosition(
AdvancedTypographicUtils.ApplyPosition(collection, index, record1);

ValueRecord record2 = class2Record.ValueRecord2;
AdvancedTypographicUtils.ApplyPosition(collection, (ushort)(index + 1), record2);
AdvancedTypographicUtils.ApplyPosition(collection, index + 1, record2);

return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
if (count <= 1)
Expand All @@ -93,7 +93,7 @@ public override bool TryUpdatePosition(
return false;
}

ushort nextIndex = (ushort)(index + 1);
int nextIndex = index + 1;
ushort nextGlyphId = collection[nextIndex];
if (nextGlyphId == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
// Mark-to-Base Attachment Positioning Subtable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
// Mark-to-Ligature Attachment Positioning.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
// Mark to mark positioning.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
ushort glyphId = collection[index];
Expand Down Expand Up @@ -130,7 +130,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
ushort glyphId = collection[index];
Expand Down Expand Up @@ -206,7 +206,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
ushort glyphId = collection[index];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
// Implements Chained Contexts Substitution, Format 1:
Expand Down Expand Up @@ -100,7 +100,7 @@ public override bool TryUpdatePosition(
SequenceLookupRecord sequenceLookupRecord = rule.SequenceLookupRecords[j];
LookupTable lookup = table.LookupList.LookupTables[sequenceLookupRecord.LookupListIndex];
ushort sequenceIndex = sequenceLookupRecord.SequenceIndex;
if (lookup.TryUpdatePosition(fontMetrics, table, collection, feature, (ushort)(index + sequenceIndex), 1))
if (lookup.TryUpdatePosition(fontMetrics, table, collection, feature, index + sequenceIndex, 1))
{
hasChanged = true;
}
Expand Down Expand Up @@ -155,7 +155,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
// Implements Chained Contexts Substitution for Format 2:
Expand Down Expand Up @@ -198,7 +198,7 @@ public override bool TryUpdatePosition(
SequenceLookupRecord sequenceLookupRecord = rule.SequenceLookupRecords[j];
LookupTable lookup = table.LookupList.LookupTables[sequenceLookupRecord.LookupListIndex];
ushort sequenceIndex = sequenceLookupRecord.SequenceIndex;
if (lookup.TryUpdatePosition(fontMetrics, table, collection, feature, (ushort)(index + sequenceIndex), 1))
if (lookup.TryUpdatePosition(fontMetrics, table, collection, feature, index + sequenceIndex, 1))
{
hasChanged = true;
}
Expand Down Expand Up @@ -249,7 +249,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
{
ushort glyphId = collection[index];
Expand All @@ -271,7 +271,7 @@ public override bool TryUpdatePosition(
ushort lookupIndex = lookupRecord.LookupListIndex;

LookupTable lookup = table.LookupList.LookupTables[lookupIndex];
if (lookup.TryUpdatePosition(fontMetrics, table, collection, feature, (ushort)(index + sequenceIndex), count - sequenceIndex))
if (lookup.TryUpdatePosition(fontMetrics, table, collection, feature, index + sequenceIndex, count - sequenceIndex))
{
hasChanged = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public override bool TryUpdatePosition(
GPosTable table,
GlyphPositioningCollection collection,
Tag feature,
ushort index,
int index,
int count)
=> false;
}
Expand Down
Loading

0 comments on commit 1b80769

Please sign in to comment.