parse first, then interpolate

[ckruse]

This avoids problems with user given values potentially breaking the parse tree

Regarding #1344

Checklist

• only relevant code is changed (make a diff before you submit the PR)
• run tests npm run test
• tests are included
• documentation is changed or added

[coveralls]

Coverage decreased (-0.2%) to 96.416% when pulling c1e4f07 on ckruse:master into 23b5481 on i18next:master.

[ckruse]

I believe this is ready for review

[adrai]

lgtm

[adrai]

should be still backwards compatible, right?
So patch or minor version is sufficient?

[jamuhl]

lgtm.... https://github.com/i18next/react-i18next/blob/master/test/trans.render.spec.js#L336 -> initial reason should be covered by tests

[adrai]

included in v11.11.3

[adrai]

Thank you @ckruse for your contribution.

[ckruse]

Thank you for your work!

[dlavrenuek]

I wanted to create an issue but since this was fixed in this PR I would like to comment with some additional information. The Trans component in react-i18next < 11.11.3 is vulnerable to XSS attacks because the provided variables were rendered as HTML. Example code:

function App() {
    const [text, setText] = useState('');

    return (
        <>
            <input
                value={text}
                onChange={(e) => setText(e.target.value)}
            />
            <p><Trans>You entered {{text}}</Trans></p>
        </>
    );
}

[dlavrenuek]

@jamuhl @adrai According to https://nodejs.org/en/security/#reporting-a-bug-in-a-third-party-module a package owner should create a security report, so the vulnerability is properly reported by npm vulnerability check and tools like dependabot.

[adrai]

@dlavrenuek
with react-i18next v11.0.0 - v11.11.4 it's always the same:
input: my <b>variable</b> <div>and other</div>
output: <p>You entered my &lt;b&gt;variable&lt;/b&gt; &lt;div&gt;and other&lt;/div&gt;</p>

Maybe you compared it to a version < v10.12.2 ? => https://github.com/i18next/react-i18next/blob/master/CHANGELOG.md#10122 (342027d)

[dlavrenuek]

@adrai I missed to provide that Trans was only vulnerable to XSS when setting the option escapeValue to true which is suggested as default by https://react.i18next.com/guides/quick-start#configure-i-18-next

interpolation: {
    escapeValue: true
}

So far I was not successful at executing javascript with it due to restrictions with transKeepBasicHtmlNodesFor, but you can destroy the page layout by adding a lot of <br /> and <p> and may be exploit it in some way.

[jamuhl]

escapeValue is suggested to be set to false -> as react already escapes strings rendered in elements.

this was tested long ago and there is yet no known way of injecting executable code into Trans - but we're happy to be proven wrong

As the transKeepBasicHtmlNodesFor come from your translation I guess that is save