Skip to content

Commit

Permalink
[MERGE #3425 @Cellule] Fix possible infinite loop in String.raw()
Browse files Browse the repository at this point in the history
Merge pull request #3425 from Cellule:raw

I was able to construct a use case where we would end up in an infinite loop.
It takes forever to run to just overflow the uint32, but I figured we should at least avoid possible infinite loops.

A possible solution could be to just cap the length to loop through to UINT32_MAX, but it would be technically not spec compliant.
  • Loading branch information
Cellule committed Jul 26, 2017
2 parents cb45a65 + 19cb89c commit 4a8dae8
Showing 1 changed file with 19 additions and 10 deletions.
29 changes: 19 additions & 10 deletions lib/Runtime/Library/JavascriptString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1560,22 +1560,31 @@ namespace Js
// strcon2 /
// expr2 \__ step 3
// strcon3 /
for (uint32 i = 1; i < length; ++i)
const auto append = [&] (Var var)
{
// First append the next substitution expression
// If we have an arg at [i+1] use that one, otherwise empty string (which is nop)
if (i+1 < args.Info.Count)
JavascriptString* string = JavascriptConversion::ToString(var, scriptContext);
stringBuilder.Append(string);
};
uint32 loopMax = length >= UINT_MAX ? UINT_MAX-1 : (uint32)length;
uint32 i = 1;
uint32 argsCount = args.Info.Count;
for (; i < loopMax; ++i)
{
// First append the next substitution expression if available
if (i + 1 < argsCount)
{
string = JavascriptConversion::ToString(args[i+1], scriptContext);

stringBuilder.Append(string);
append(args[i + 1]);
}

// Then append the next string (this will also cover the final string case)
var = JavascriptOperators::OP_GetElementI_UInt32(raw, i, scriptContext);
string = JavascriptConversion::ToString(var, scriptContext);
append(JavascriptOperators::OP_GetElementI_UInt32(raw, i, scriptContext));
}

stringBuilder.Append(string);
// Length can be greater than uint32 max (unlikely in practice)
for (int64 j = (int64)i; j < length; ++j)
{
// Append whatever is left in the array/object
append(JavascriptOperators::OP_GetElementI(raw, JavascriptNumber::ToVar(j, scriptContext), scriptContext));
}

// CompoundString::Builder has saved our lives
Expand Down

0 comments on commit 4a8dae8

Please sign in to comment.