From 13bc899db148746087d40d199836789354e63900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Wed, 17 Jul 2019 15:09:56 +0200 Subject: [PATCH 1/2] Return FluentNone from Patterns with too long placeables --- fluent/src/resolver.js | 7 ++++--- fluent/test/bomb_test.js | 13 ++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/fluent/src/resolver.js b/fluent/src/resolver.js index 16e087bb7..c3614c82f 100644 --- a/fluent/src/resolver.js +++ b/fluent/src/resolver.js @@ -289,11 +289,12 @@ export function resolveComplexPattern(scope, ptn) { `(${part.length}, max allowed is ${MAX_PLACEABLE_LENGTH})` ) ); - result.push(part.slice(MAX_PLACEABLE_LENGTH)); - } else { - result.push(part); + scope.dirty.delete(ptn); + return new FluentNone(); } + result.push(part); + if (useIsolating) { result.push(PDI); } diff --git a/fluent/test/bomb_test.js b/fluent/test/bomb_test.js index 160193390..bcf6f9a9c 100644 --- a/fluent/test/bomb_test.js +++ b/fluent/test/bomb_test.js @@ -13,6 +13,8 @@ suite('Reference bombs', function() { }); suite('Billion Laughs', function(){ + this.timeout(10000); + suiteSetup(function() { bundle = new FluentBundle('en-US', { useIsolating: false }); bundle.addMessages(ftl` @@ -30,13 +32,14 @@ suite('Reference bombs', function() { `); }); - // XXX Protect the FTL Resolver against the billion laughs attack - // https://bugzil.la/1307126 - test.skip('does not expand all placeables', function() { + test('does not expand all placeables', function() { const msg = bundle.getMessage('lolz'); const val = bundle.formatPattern(msg.value, args, errs); - assert.strictEqual(val, '???'); - assert.strictEqual(errs.length, 1); + assert.strictEqual( + val, + '{???} {???} {???} {???} {???} {???} {???} {???} {???} {???}' + ); + assert.strictEqual(errs.length, 10010); }); }); }); From aec37fa4548e5511957ddc959619d3d6300d5c15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=C5=9B=20Ma=C5=82olepszy?= Date: Thu, 18 Jul 2019 11:29:53 +0200 Subject: [PATCH 2/2] Instantly bail out from the resolver on too long placeables --- fluent/src/bundle.js | 10 ++++++++-- fluent/src/resolver.js | 15 ++++++++------- fluent/test/bomb_test.js | 9 ++------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/fluent/src/bundle.js b/fluent/src/bundle.js index 7b185d5b7..79f1cb496 100644 --- a/fluent/src/bundle.js +++ b/fluent/src/bundle.js @@ -1,5 +1,6 @@ import {resolveComplexPattern} from "./resolver.js"; import FluentResource from "./resource.js"; +import { FluentNone } from "./types.js"; /** * Message bundles are single-language stores of translations. They are @@ -220,8 +221,13 @@ export default class FluentBundle { // Resolve a complex pattern. if (Array.isArray(pattern)) { let scope = this._createScope(args, errors); - let value = resolveComplexPattern(scope, pattern); - return value.toString(scope); + try { + let value = resolveComplexPattern(scope, pattern); + return value.toString(scope); + } catch (err) { + scope.errors.push(err); + return new FluentNone().toString(scope); + } } throw new TypeError("Invalid Pattern type"); diff --git a/fluent/src/resolver.js b/fluent/src/resolver.js index c3614c82f..f2d3fa3d8 100644 --- a/fluent/src/resolver.js +++ b/fluent/src/resolver.js @@ -283,14 +283,15 @@ export function resolveComplexPattern(scope, ptn) { } if (part.length > MAX_PLACEABLE_LENGTH) { - scope.errors.push( - new RangeError( - "Too many characters in placeable " + - `(${part.length}, max allowed is ${MAX_PLACEABLE_LENGTH})` - ) - ); scope.dirty.delete(ptn); - return new FluentNone(); + // This is a fatal error which causes the resolver to instantly bail out + // on this pattern. The length check protects against excessive memory + // usage, and throwing protects against eating up the CPU when long + // placeables are deeply nested. + throw new RangeError( + "Too many characters in placeable " + + `(${part.length}, max allowed is ${MAX_PLACEABLE_LENGTH})` + ); } result.push(part); diff --git a/fluent/test/bomb_test.js b/fluent/test/bomb_test.js index bcf6f9a9c..b5135f5de 100644 --- a/fluent/test/bomb_test.js +++ b/fluent/test/bomb_test.js @@ -13,8 +13,6 @@ suite('Reference bombs', function() { }); suite('Billion Laughs', function(){ - this.timeout(10000); - suiteSetup(function() { bundle = new FluentBundle('en-US', { useIsolating: false }); bundle.addMessages(ftl` @@ -35,11 +33,8 @@ suite('Reference bombs', function() { test('does not expand all placeables', function() { const msg = bundle.getMessage('lolz'); const val = bundle.formatPattern(msg.value, args, errs); - assert.strictEqual( - val, - '{???} {???} {???} {???} {???} {???} {???} {???} {???} {???}' - ); - assert.strictEqual(errs.length, 10010); + assert.strictEqual(val, '{???}'); + assert.strictEqual(errs.length, 1); }); }); });