Skip to content

Commit 62023d2

Browse files
security(lottie): escape user-controlled values in HTML renderer iframe
Co-authored-by: Sharjeel Yunus <sharjeelyunus@users.noreply.github.com>
1 parent 6d1aeae commit 62023d2

3 files changed

Lines changed: 76 additions & 2 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import 'dart:convert';
2+
3+
import 'package:meta/meta.dart';
4+
5+
/// Returns true when [id] is safe to embed in generated JavaScript identifiers
6+
/// and HTML attributes for the Lottie HTML renderer iframe.
7+
@visibleForTesting
8+
bool isSafeLottieHtmlRendererId(String id) {
9+
if (id.isEmpty || id.length > 128) {
10+
return false;
11+
}
12+
return RegExp(r'^[a-zA-Z][a-zA-Z0-9_]*$').hasMatch(id);
13+
}
14+
15+
/// Sanitizes [id] for embedding in generated iframe JavaScript.
16+
///
17+
/// Falls back to [fallbackId] when [id] contains characters that would break
18+
/// out of JS string literals or variable names.
19+
@visibleForTesting
20+
String sanitizeLottieHtmlRendererId(String id, String fallbackId) {
21+
return isSafeLottieHtmlRendererId(id) ? id : fallbackId;
22+
}
23+
24+
/// JSON-encoded source URL literal safe to embed in generated iframe JS.
25+
@visibleForTesting
26+
String lottieSourceLiteral(String source) => jsonEncode(source);

modules/ensemble/lib/widget/lottie/web/lottiestate.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:ensemble/util/utils.dart';
1010
import 'package:ensemble/widget/helpers/box_wrapper.dart';
1111
import 'package:ensemble/widget/helpers/widgets.dart';
1212
import 'package:ensemble/widget/lottie/lottie.dart';
13+
import 'package:ensemble/widget/lottie/lottie_html_renderer_security.dart';
1314
import 'package:ensemble/widget/widget_util.dart';
1415
import 'package:flutter/material.dart';
1516
import 'package:flutter/widgets.dart';
@@ -29,7 +30,7 @@ class LottieState extends EWidgetState<EnsembleLottie>
2930
@override
3031
void initState() {
3132
super.initState();
32-
divId = widget.controller.id ?? id;
33+
divId = sanitizeLottieHtmlRendererId(widget.controller.id ?? id, id);
3334
if (isCanvasKit) {
3435
widget.controller
3536
..lottieController = AnimationController(vsync: this)
@@ -48,6 +49,7 @@ class LottieState extends EWidgetState<EnsembleLottie>
4849
void didUpdateWidget(covariant EnsembleLottie oldWidget) {
4950
super.didUpdateWidget(oldWidget);
5051
widget.controller.lottieAction = this;
52+
divId = sanitizeLottieHtmlRendererId(widget.controller.id ?? id, id);
5153
}
5254

5355
@override
@@ -115,6 +117,7 @@ class LottieState extends EWidgetState<EnsembleLottie>
115117
// the image will throw exception. We have to use a permanent placeholder
116118
// until the binding engages
117119
// HTML & JS code for the web html renderer
120+
final sourceLiteral = lottieSourceLiteral(source);
118121
final htmlString = '''
119122
<html>
120123
<body>
@@ -129,7 +132,7 @@ class LottieState extends EWidgetState<EnsembleLottie>
129132
let player_$divId = document.getElementById("$divId");
130133
// A counter variable which increments upon each event and thus making each event unique and allowing to segregate from old events
131134
let counter = 0;
132-
player_$divId.load("$source");
135+
player_$divId.load($sourceLiteral);
133136
if ($autoPlay) player_$divId.play();
134137
window.parent.addEventListener("message", handleMessage, false); // Hooking the event listener
135138
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import 'dart:convert';
2+
3+
import 'package:ensemble/widget/lottie/lottie_html_renderer_security.dart';
4+
import 'package:flutter_test/flutter_test.dart';
5+
6+
void main() {
7+
group('isSafeLottieHtmlRendererId', () {
8+
test('accepts simple alphanumeric ids', () {
9+
expect(isSafeLottieHtmlRendererId('lottie_123'), isTrue);
10+
expect(isSafeLottieHtmlRendererId('myWidget'), isTrue);
11+
});
12+
13+
test('rejects ids that can break generated JavaScript', () {
14+
expect(isSafeLottieHtmlRendererId('foo"); alert(1);//'), isFalse);
15+
expect(isSafeLottieHtmlRendererId('foo-bar'), isFalse);
16+
expect(isSafeLottieHtmlRendererId(''), isFalse);
17+
expect(isSafeLottieHtmlRendererId('a' * 129), isFalse);
18+
});
19+
});
20+
21+
group('sanitizeLottieHtmlRendererId', () {
22+
test('falls back when id is unsafe', () {
23+
expect(
24+
sanitizeLottieHtmlRendererId('bad"); alert(1);//', 'lottie_safe'),
25+
'lottie_safe',
26+
);
27+
});
28+
});
29+
30+
group('lottieSourceLiteral', () {
31+
test('JSON-encodes source URLs for safe embedding', () {
32+
expect(
33+
lottieSourceLiteral('https://cdn.example.com/anim.json'),
34+
jsonEncode('https://cdn.example.com/anim.json'),
35+
);
36+
});
37+
38+
test('neutralizes JavaScript breakout in source URL', () {
39+
const malicious = '"); alert(1);//';
40+
final literal = lottieSourceLiteral(malicious);
41+
expect(literal, jsonEncode(malicious));
42+
expect(literal, isNot(contains('load(""); alert(1);')));
43+
});
44+
});
45+
}

0 commit comments

Comments
 (0)