diff --git a/packages/vector_graphics_compiler/CHANGELOG.md b/packages/vector_graphics_compiler/CHANGELOG.md index 2d2316afdcac..de90ba4cc8c6 100644 --- a/packages/vector_graphics_compiler/CHANGELOG.md +++ b/packages/vector_graphics_compiler/CHANGELOG.md @@ -1,5 +1,7 @@ -## NEXT +## 1.2.4 +* Fix Stack Overflow crashes caused by circular references (masks, patterns, deferred nodes, and clip paths). +* Prevent CPU/Memory Denial of Service (DoS) resource exhaustion from exponential DAG reference expansions (Billion Laughs SVG exploits) by enforcing a strict, cumulative reference expansion safety limit of 1,000. * Updates minimum supported SDK version to Flutter 3.38/Dart 3.10. ## 1.2.3 diff --git a/packages/vector_graphics_compiler/lib/src/svg/constants.dart b/packages/vector_graphics_compiler/lib/src/svg/constants.dart new file mode 100644 index 000000000000..0f1cf3621eb3 --- /dev/null +++ b/packages/vector_graphics_compiler/lib/src/svg/constants.dart @@ -0,0 +1,10 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +/// The maximum number of nested reference expansions allowed in an SVG to prevent DoS exploits. +const int kMaxReferenceExpansions = 1000; + +/// The error message thrown when the nested reference expansions limit is exceeded. +const String kMaxReferenceExpansionsErrorMessage = + 'SVG contains too many nested reference expansions (possible Denial of Service exploit).'; diff --git a/packages/vector_graphics_compiler/lib/src/svg/parser.dart b/packages/vector_graphics_compiler/lib/src/svg/parser.dart index c0d1c5555f0d..d32b060d854d 100644 --- a/packages/vector_graphics_compiler/lib/src/svg/parser.dart +++ b/packages/vector_graphics_compiler/lib/src/svg/parser.dart @@ -21,6 +21,7 @@ import '../vector_instructions.dart'; import 'clipping_optimizer.dart'; import 'color_mapper.dart'; import 'colors.dart'; +import 'constants.dart'; import 'masking_optimizer.dart'; import 'node.dart'; import 'numbers.dart' as numbers show parseDoubleWithUnits; @@ -1678,6 +1679,7 @@ class _Resolver { final Map _drawables = {}; final Map _shaders = {}; final Map> _clips = >{}; + int _deferredExpansionCount = 0; bool _sealed = false; @@ -1702,6 +1704,7 @@ class _Resolver { final pathBuilders = []; PathBuilder? currentPath; + final activeDeferred = {}; void extractPathsFromNode(Node? target) { if (target is PathNode) { final nextPath = PathBuilder.fromPath(target.path); @@ -1716,7 +1719,19 @@ class _Resolver { currentPath!.addPath(nextPath.toPath(reset: false)); } } else if (target is DeferredNode) { - extractPathsFromNode(target.resolver(target.refId)); + _deferredExpansionCount++; + if (_deferredExpansionCount > kMaxReferenceExpansions) { + throw StateError(kMaxReferenceExpansionsErrorMessage); + } + if (!activeDeferred.add(target.refId)) { + // Recursive loop detected. + return; + } + try { + extractPathsFromNode(target.resolver(target.refId)); + } finally { + activeDeferred.remove(target.refId); + } } else if (target is ParentNode) { target.visitChildren(extractPathsFromNode); } diff --git a/packages/vector_graphics_compiler/lib/src/svg/resolver.dart b/packages/vector_graphics_compiler/lib/src/svg/resolver.dart index c36266356f4b..a5abbdfa2c29 100644 --- a/packages/vector_graphics_compiler/lib/src/svg/resolver.dart +++ b/packages/vector_graphics_compiler/lib/src/svg/resolver.dart @@ -9,6 +9,7 @@ import '../geometry/path.dart'; import '../geometry/vertices.dart'; import '../image/image_info.dart'; import '../paint.dart'; +import 'constants.dart'; import 'node.dart'; import 'parser.dart'; import 'visitor.dart'; @@ -19,6 +20,11 @@ import 'visitor.dart'; class ResolvingVisitor extends Visitor { late Rect _bounds; + final Set _activeMasks = {}; + final Set _activeDeferred = {}; + final Set _activePatterns = {}; + int _deferredExpansionCount = 0; + @override Node visitClipNode(ClipNode clipNode, AffineMatrix data) { final AffineMatrix childTransform = clipNode.concatTransform(data); @@ -37,19 +43,31 @@ class ResolvingVisitor extends Visitor { @override Node visitMaskNode(MaskNode maskNode, AffineMatrix data) { - final AttributedNode? resolvedMask = maskNode.resolver(maskNode.maskId); - if (resolvedMask == null) { + _deferredExpansionCount++; + if (_deferredExpansionCount > kMaxReferenceExpansions) { + throw StateError(kMaxReferenceExpansionsErrorMessage); + } + if (!_activeMasks.add(maskNode.maskId)) { + // Recursive loop detected. return maskNode.child.accept(this, data); } - final Node child = maskNode.child.accept(this, data); - final AffineMatrix childTransform = maskNode.concatTransform(data); - final Node mask = resolvedMask.accept(this, childTransform); - - return ResolvedMaskNode( - child: child, - mask: mask, - blendMode: maskNode.blendMode, - ); + try { + final AttributedNode? resolvedMask = maskNode.resolver(maskNode.maskId); + if (resolvedMask == null) { + return maskNode.child.accept(this, data); + } + final Node child = maskNode.child.accept(this, data); + final AffineMatrix childTransform = maskNode.concatTransform(data); + final Node mask = resolvedMask.accept(this, childTransform); + + return ResolvedMaskNode( + child: child, + mask: mask, + blendMode: maskNode.blendMode, + ); + } finally { + _activeMasks.remove(maskNode.maskId); + } } @override @@ -180,17 +198,29 @@ class ResolvingVisitor extends Visitor { @override Node visitDeferredNode(DeferredNode deferredNode, AffineMatrix data) { - final AttributedNode? resolvedNode = deferredNode.resolver( - deferredNode.refId, - ); - if (resolvedNode == null) { + _deferredExpansionCount++; + if (_deferredExpansionCount > kMaxReferenceExpansions) { + throw StateError(kMaxReferenceExpansionsErrorMessage); + } + if (!_activeDeferred.add(deferredNode.refId)) { + // Recursive loop detected. return Node.empty; } - final Node concreteRef = resolvedNode.applyAttributes( - deferredNode.attributes, - replace: true, - ); - return concreteRef.accept(this, data); + try { + final AttributedNode? resolvedNode = deferredNode.resolver( + deferredNode.refId, + ); + if (resolvedNode == null) { + return Node.empty; + } + final Node concreteRef = resolvedNode.applyAttributes( + deferredNode.attributes, + replace: true, + ); + return concreteRef.accept(this, data); + } finally { + _activeDeferred.remove(deferredNode.refId); + } } @override @@ -293,26 +323,38 @@ class ResolvingVisitor extends Visitor { @override Node visitPatternNode(PatternNode patternNode, AffineMatrix data) { - final AttributedNode? resolvedPattern = patternNode.resolver( - patternNode.patternId, - ); - if (resolvedPattern == null) { + _deferredExpansionCount++; + if (_deferredExpansionCount > kMaxReferenceExpansions) { + throw StateError(kMaxReferenceExpansionsErrorMessage); + } + if (!_activePatterns.add(patternNode.patternId)) { + // Recursive loop detected. return patternNode.child.accept(this, data); } - final Node child = patternNode.child.accept(this, data); - final AffineMatrix childTransform = patternNode.concatTransform(data); - final Node pattern = resolvedPattern.accept(this, childTransform); - - return ResolvedPatternNode( - child: child, - pattern: pattern, - x: resolvedPattern.attributes.x?.calculate(0) ?? 0, - y: resolvedPattern.attributes.y?.calculate(0) ?? 0, - width: resolvedPattern.attributes.width!, - height: resolvedPattern.attributes.height!, - transform: data, - id: patternNode.patternId, - ); + try { + final AttributedNode? resolvedPattern = patternNode.resolver( + patternNode.patternId, + ); + if (resolvedPattern == null) { + return patternNode.child.accept(this, data); + } + final Node child = patternNode.child.accept(this, data); + final AffineMatrix childTransform = patternNode.concatTransform(data); + final Node pattern = resolvedPattern.accept(this, childTransform); + + return ResolvedPatternNode( + child: child, + pattern: pattern, + x: resolvedPattern.attributes.x?.calculate(0) ?? 0, + y: resolvedPattern.attributes.y?.calculate(0) ?? 0, + width: resolvedPattern.attributes.width!, + height: resolvedPattern.attributes.height!, + transform: data, + id: patternNode.patternId, + ); + } finally { + _activePatterns.remove(patternNode.patternId); + } } @override diff --git a/packages/vector_graphics_compiler/pubspec.yaml b/packages/vector_graphics_compiler/pubspec.yaml index 7c89a4111110..040e7d5af6f8 100644 --- a/packages/vector_graphics_compiler/pubspec.yaml +++ b/packages/vector_graphics_compiler/pubspec.yaml @@ -2,7 +2,7 @@ name: vector_graphics_compiler description: A compiler to convert SVGs to the binary format used by `package:vector_graphics`. repository: https://github.com/flutter/packages/tree/main/packages/vector_graphics_compiler issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_graphics%22 -version: 1.2.3 +version: 1.2.4 executables: vector_graphics_compiler: diff --git a/packages/vector_graphics_compiler/test/parser_test.dart b/packages/vector_graphics_compiler/test/parser_test.dart index 934cee62ac52..2b9409830b70 100644 --- a/packages/vector_graphics_compiler/test/parser_test.dart +++ b/packages/vector_graphics_compiler/test/parser_test.dart @@ -29,6 +29,346 @@ class _TestOpacityColorMapper implements ColorMapper { } void main() { + test('Exponential DAG expansion triggers DoS protection limit', () { + final svg = + ''' + + + + +${[for (var i = 2; i <= 30; i++) ' '].join('\n')} + + +'''; + + expect( + () => parseWithoutOptimizers(svg), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('SVG contains too many nested reference expansions'), + ), + ), + ); + }); + + test('Exponential DAG clipPath expansion triggers DoS protection limit', () { + final svg = + ''' + + + + +${[for (var i = 2; i <= 30; i++) ' '].join('\n')} + + + +'''; + + expect( + () => parseWithoutOptimizers(svg), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('SVG contains too many nested reference expansions'), + ), + ), + ); + }); + + test( + 'Cumulative clipPath reference expansions trigger DoS protection limit', + () { + final svg = + ''' + + + + +${[for (var i = 2; i <= 8; i++) ' '].join('\n')} + + + + + + +'''; + + expect( + () => parseWithoutOptimizers(svg), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('SVG contains too many nested reference expansions'), + ), + ), + ); + }, + ); + + test('Exponential DAG mask expansion triggers DoS protection limit', () { + final svg = + ''' + + + + +${[for (var i = 2; i <= 30; i++) ' '].join('\n')} + + +'''; + + expect( + () => parseWithoutOptimizers(svg), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('SVG contains too many nested reference expansions'), + ), + ), + ); + }); + + test('Exponential DAG pattern expansion triggers DoS protection limit', () { + final svg = + ''' + + + + +${[for (var i = 2; i <= 30; i++) ' '].join('\n')} + + +'''; + + expect( + () => parseWithoutOptimizers(svg), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('SVG contains too many nested reference expansions'), + ), + ), + ); + }); + + test('Circular Mask Loop Avoidance', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + +'''); + + expect(instructions.paths.isNotEmpty, true); + }); + + test('Unreferenced Circular Mask Loop resolves successfully', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + + + + + + + +'''); + + expect(instructions.paths.length, 1); + }); + + test('Multi-hop Referenced Circular Mask Loop Avoidance', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + + + + + + + + + + + + +'''); + + expect(instructions.paths.isNotEmpty, true); + }); + + test('Circular Deferred Node Loop Avoidance', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + +'''); + + expect(instructions.paths.isEmpty, true); + }); + + test('Unreferenced Circular Use Loop resolves successfully', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + + + +'''); + + expect(instructions.paths.length, 1); + }); + + test('Multi-hop Referenced Circular Use Loop Avoidance', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + + + + + + +'''); + + expect(instructions.paths.isEmpty, true); + }); + + test('Circular Pattern Loop Avoidance', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + +'''); + + expect(instructions.paths.isNotEmpty, true); + }); + + test('Circular ClipPath Loop Avoidance', () { + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + + + + +'''); + expect(instructions.paths.length, 1); + expect( + instructions.commands.any((c) => c.type == DrawCommandType.clip), + false, + ); + }); + + test('Shared DAG Sibling Node (No Loop) resolves successfully', () { + // Case 1: Identical positions. The paths are geometrically identical, + // so they are deduplicated in the unique paths list, but drawn twice in the command list. + final VectorInstructions instructions = parseWithoutOptimizers(''' + + + + + + + + + + + + + + + + +'''); + + expect(instructions.paths.length, 1); + expect( + instructions.commands.where((c) => c.type == DrawCommandType.path).length, + 2, + ); + + // Case 2: Distinct positions. The paths are transformed differently, + // so they are distinct paths and both exist in the unique paths list. + final VectorInstructions distinctInstructions = parseWithoutOptimizers(''' + + + + + + + + + + + + + + + + +'''); + + expect(distinctInstructions.paths.length, 2); + expect( + distinctInstructions.commands + .where((c) => c.type == DrawCommandType.path) + .length, + 2, + ); + }); + test('Reuse ID self-referentially', () { final VectorInstructions instructions = parseWithoutOptimizers('''