search

ChapelR / custom-macros-for-sugarcube-2

A collection of systems and macros for Twine.
https://macros.twinelab.net/
The Unlicense
170 stars 43 forks source link

[Pronoun Templates] Potentially unsafe string test in isUpper() #25

Closed tmedwards closed 5 years ago

tmedwards commented 5 years ago

https://github.com/ChapelR/custom-macros-for-sugarcube-2/blob/53a9b2388261c2d63a2e1abaceb559fab42eb634/scripts/pronouns.js#L213

There a potentially unsafe string test in the isUpper() function of your pronoun templates set. The test (seen above) could fail if name starts with a non-BMP code point—meaning it would consist of two code units. a surrogate pair, rather than one. Since JavaScript strings expose UTF-16 code units, rather than code points, the test as written would be checking only the first code unit in the surrogate pair.

The following code, which uses SugarCube's <String>.first() extension method, handles both BMP and non-BMP code points correctly:

        if (name.first() === name.first().toUpperCase()) {
tmedwards commented 5 years ago

This isn't that serious, actually. Looking closer, as I should have done before submitting the issue, you define the template names and you're obviously not using non-BMP code points to start them. Thus, the only time this might be a issue is if a user edits in their own template names, which while possible is probably unlikely.