Skip to content

Commit 85d42e7

Browse files
MikeHolmanleirocks
MikeHolman
authored and
leirocks
committed
[CVE-2017-11861] [ChakraCore] Chakra JIT - Incorrect integer overflow check in Lowerer::LowerBoundCheck - Google, Inc.
Math on IntConstType should be bounded by IRType of the Opnd. In case of Lowerer::LowerBoundCheck, it ended up that the IntConstOpnd is a TyInt32 and the overflow leads to bad bound check being emitted. For this I added a new IntConstMath class which takes an IRType as a parameter and validates that the result can be represented by that IRType.
1 parent c1bdfff commit 85d42e7

13 files changed

Lines changed: 144 additions & 34 deletions

‎lib/Backend/Backend.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ enum IRDumpFlags
139139
#include "SymTable.h"
140140
#include "IR.h"
141141
#include "Opnd.h"
142+
#include "IntConstMath.h"
142143
#include "IntOverflowDoesNotMatterRange.h"
143144
#include "IntConstantBounds.h"
144145
#include "ValueRelativeOffset.h"

‎lib/Backend/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ add_library (Chakra.Backend OBJECT
3939
InliningDecider.cpp
4040
InliningHeuristics.cpp
4141
IntBounds.cpp
42+
IntConstMath.cpp
4243
InterpreterThunkEmitter.cpp
4344
JITThunkEmitter.cpp
4445
JITOutput.cpp

‎lib/Backend/Chakra.Backend.vcxproj‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@
218218
<ClCompile Include="$(MSBuildThisFileDirectory)GlobOptBlockData.cpp" />
219219
<ClCompile Include="$(MSBuildThisFileDirectory)ValueInfo.cpp" />
220220
<ClCompile Include="$(MSBuildThisFileDirectory)JITThunkEmitter.cpp" />
221+
<ClCompile Include="$(MSBuildThisFileDirectory)IntConstMath.cpp" />
221222
</ItemGroup>
222223
<ItemGroup>
223224
<ClInclude Include="AgenPeeps.h" />
@@ -259,6 +260,7 @@
259260
<ClInclude Include="FunctionJITRuntimeInfo.h" />
260261
<ClInclude Include="FunctionJITTimeInfo.h" />
261262
<ClInclude Include="GlobOptBlockData.h" />
263+
<ClInclude Include="IntConstMath.h" />
262264
<ClInclude Include="IRBaseTypeList.h" />
263265
<ClInclude Include="IRBuilderAsmJs.h" />
264266
<ClInclude Include="BackendOpCodeAttrAsmJs.h" />

‎lib/Backend/Chakra.Backend.vcxproj.filters‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@
130130
<ClCompile Include="$(MSBuildThisFileDirectory)GlobOptBlockData.cpp" />
131131
<ClCompile Include="$(MSBuildThisFileDirectory)ValueInfo.cpp" />
132132
<ClCompile Include="$(MSBuildThisFileDirectory)JITThunkEmitter.cpp" />
133+
<ClCompile Include="$(MSBuildThisFileDirectory)IntConstMath.cpp" />
133134
</ItemGroup>
134135
<ItemGroup>
135136
<ClInclude Include="AgenPeeps.h" />
@@ -345,6 +346,7 @@
345346
<ClInclude Include="GlobOptBlockData.h" />
346347
<ClInclude Include="ValueInfo.h" />
347348
<ClInclude Include="JITThunkEmitter.h" />
349+
<ClInclude Include="IntConstMath.h" />
348350
</ItemGroup>
349351
<ItemGroup>
350352
<MASM Include="$(MSBuildThisFileDirectory)amd64\LinearScanMdA.asm">

‎lib/Backend/GlobOpt.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12351,7 +12351,7 @@ GlobOpt::OptConstFoldBinary(
1235112351
}
1235212352

1235312353
IntConstType tmpValueOut;
12354-
if (!instr->BinaryCalculator(src1IntConstantValue, src2IntConstantValue, &tmpValueOut)
12354+
if (!instr->BinaryCalculator(src1IntConstantValue, src2IntConstantValue, &tmpValueOut, TyInt32)
1235512355
|| !Math::FitsInDWord(tmpValueOut))
1235612356
{
1235712357
return false;

‎lib/Backend/IR.cpp‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3807,28 +3807,28 @@ bool Instr::BinaryCalculatorT(T src1Const, T src2Const, int64 *pResult, bool che
38073807
template bool Instr::BinaryCalculatorT<int>(int src1Const64, int src2Const64, int64 *pResult, bool checkWouldTrap);
38083808
template bool Instr::BinaryCalculatorT<int64>(int64 src1Const64, int64 src2Const64, int64 *pResult, bool checkWouldTrap);
38093809

3810-
bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, IntConstType *pResult)
3810+
bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, IntConstType *pResult, IRType type)
38113811
{
38123812
IntConstType value = 0;
38133813

38143814
switch (this->m_opcode)
38153815
{
38163816
case Js::OpCode::Add_A:
3817-
if (IntConstMath::Add(src1Const, src2Const, &value))
3817+
if (IntConstMath::Add(src1Const, src2Const, type, &value))
38183818
{
38193819
return false;
38203820
}
38213821
break;
38223822

38233823
case Js::OpCode::Sub_A:
3824-
if (IntConstMath::Sub(src1Const, src2Const, &value))
3824+
if (IntConstMath::Sub(src1Const, src2Const, type, &value))
38253825
{
38263826
return false;
38273827
}
38283828
break;
38293829

38303830
case Js::OpCode::Mul_A:
3831-
if (IntConstMath::Mul(src1Const, src2Const, &value))
3831+
if (IntConstMath::Mul(src1Const, src2Const, type, &value))
38323832
{
38333833
return false;
38343834
}
@@ -3852,7 +3852,7 @@ bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, Int
38523852
// folds to -0. Bail for now...
38533853
return false;
38543854
}
3855-
if (IntConstMath::Div(src1Const, src2Const, &value))
3855+
if (IntConstMath::Div(src1Const, src2Const, type, &value))
38563856
{
38573857
return false;
38583858
}
@@ -3870,7 +3870,7 @@ bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, Int
38703870
// Bail for now...
38713871
return false;
38723872
}
3873-
if (IntConstMath::Mod(src1Const, src2Const, &value))
3873+
if (IntConstMath::Mod(src1Const, src2Const, type, &value))
38743874
{
38753875
return false;
38763876
}
@@ -3884,17 +3884,15 @@ bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, Int
38843884

38853885
case Js::OpCode::Shl_A:
38863886
// We don't care about overflow here
3887-
IntConstMath::Shl(src1Const, src2Const & 0x1F, &value);
3887+
value = src1Const << (src2Const & 0x1F);
38883888
break;
38893889

38903890
case Js::OpCode::Shr_A:
3891-
// We don't care about overflow here, and there shouldn't be any
3892-
IntConstMath::Shr(src1Const, src2Const & 0x1F, &value);
3891+
value = src1Const >> (src2Const & 0x1F);
38933892
break;
38943893

38953894
case Js::OpCode::ShrU_A:
3896-
// We don't care about overflow here, and there shouldn't be any
3897-
IntConstMath::ShrU(src1Const, src2Const & 0x1F, &value);
3895+
value = ((UIntConstType)src1Const) >> (src2Const & 0x1F);
38983896
if (value < 0)
38993897
{
39003898
// ShrU produces a UInt32. If it doesn't fit in an Int32, bail as we don't
@@ -3904,18 +3902,15 @@ bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, Int
39043902
break;
39053903

39063904
case Js::OpCode::And_A:
3907-
// We don't care about overflow here, and there shouldn't be any
3908-
IntConstMath::And(src1Const, src2Const, &value);
3905+
value = src1Const & src2Const;
39093906
break;
39103907

39113908
case Js::OpCode::Or_A:
3912-
// We don't care about overflow here, and there shouldn't be any
3913-
IntConstMath::Or(src1Const, src2Const, &value);
3909+
value = src1Const | src2Const;
39143910
break;
39153911

39163912
case Js::OpCode::Xor_A:
3917-
// We don't care about overflow here, and there shouldn't be any
3918-
IntConstMath::Xor(src1Const, src2Const, &value);
3913+
value = src1Const ^ src2Const;
39193914
break;
39203915

39213916
case Js::OpCode::InlineMathMin:
@@ -3935,7 +3930,7 @@ bool Instr::BinaryCalculator(IntConstType src1Const, IntConstType src2Const, Int
39353930
return true;
39363931
}
39373932

3938-
bool Instr::UnaryCalculator(IntConstType src1Const, IntConstType *pResult)
3933+
bool Instr::UnaryCalculator(IntConstType src1Const, IntConstType *pResult, IRType type)
39393934
{
39403935
IntConstType value = 0;
39413936

@@ -3948,14 +3943,14 @@ bool Instr::UnaryCalculator(IntConstType src1Const, IntConstType *pResult)
39483943
return false;
39493944
}
39503945

3951-
if (IntConstMath::Neg(src1Const, &value))
3946+
if (IntConstMath::Neg(src1Const, type, &value))
39523947
{
39533948
return false;
39543949
}
39553950
break;
39563951

39573952
case Js::OpCode::Not_A:
3958-
IntConstMath::Not(src1Const, &value);
3953+
value = ~src1Const;
39593954
break;
39603955

39613956
case Js::OpCode::Ld_A:
@@ -3973,14 +3968,14 @@ bool Instr::UnaryCalculator(IntConstType src1Const, IntConstType *pResult)
39733968
break;
39743969

39753970
case Js::OpCode::Incr_A:
3976-
if (IntConstMath::Inc(src1Const, &value))
3971+
if (IntConstMath::Inc(src1Const, type, &value))
39773972
{
39783973
return false;
39793974
}
39803975
break;
39813976

39823977
case Js::OpCode::Decr_A:
3983-
if (IntConstMath::Dec(src1Const, &value))
3978+
if (IntConstMath::Dec(src1Const, type, &value))
39843979
{
39853980
return false;
39863981
}

‎lib/Backend/IR.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,10 @@ class Instr
334334
bool IsCmCC_R8();
335335
bool IsCmCC_I4();
336336
bool IsNeq();
337-
bool BinaryCalculator(IntConstType src1Const, IntConstType src2Const, IntConstType *pResult);
337+
bool BinaryCalculator(IntConstType src1Const, IntConstType src2Const, IntConstType *pResult, IRType type);
338338
template <typename T>
339339
bool BinaryCalculatorT(T src1Const, T src2Const, int64 *pResult, bool checkWouldTrap);
340-
bool UnaryCalculator(IntConstType src1Const, IntConstType *pResult);
340+
bool UnaryCalculator(IntConstType src1Const, IntConstType *pResult, IRType type);
341341
IR::Instr* GetNextArg();
342342

343343
// Iterates argument chain

‎lib/Backend/Inline.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4419,7 +4419,7 @@ bool Inline::InlConstFold(IR::Instr *instr, IntConstType *pValue, __in_ecount_op
44194419
{
44204420
IntConstType src2Constant = *pValue;
44214421

4422-
if (!instr->BinaryCalculator(src1Constant, src2Constant, pValue)
4422+
if (!instr->BinaryCalculator(src1Constant, src2Constant, pValue, TyInt32)
44234423
|| !Math::FitsInDWord(*pValue))
44244424
{
44254425
return false;
@@ -4457,7 +4457,7 @@ bool Inline::InlConstFold(IR::Instr *instr, IntConstType *pValue, __in_ecount_op
44574457
}
44584458
else
44594459
{
4460-
if (!instr->UnaryCalculator(src1Constant, pValue)
4460+
if (!instr->UnaryCalculator(src1Constant, pValue, TyInt32)
44614461
|| !Math::FitsInDWord(*pValue))
44624462
{
44634463
// Skip over BytecodeArgOutCapture

‎lib/Backend/IntBounds.cpp‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -743,22 +743,25 @@ bool IntBoundCheck::SetBoundOffset(const int offset, const bool isLoopCountBased
743743
// Determine the previous offset from the instruction (src1 <= src2 + dst)
744744
IR::IntConstOpnd *dstOpnd = nullptr;
745745
IntConstType previousOffset = 0;
746+
IRType offsetType = TyMachReg;
746747
if (instr->GetDst())
747748
{
748749
dstOpnd = instr->GetDst()->AsIntConstOpnd();
749750
previousOffset = dstOpnd->GetValue();
751+
offsetType = dstOpnd->GetType();
750752
}
751753

752754
IR::IntConstOpnd *src1Opnd = nullptr;
753755
if (instr->GetSrc1()->IsIntConstOpnd())
754756
{
755757
src1Opnd = instr->GetSrc1()->AsIntConstOpnd();
756-
if (IntConstMath::Sub(previousOffset, src1Opnd->GetValue(), &previousOffset))
758+
if (IntConstMath::Sub(previousOffset, src1Opnd->GetValue(), src1Opnd->GetType(), &previousOffset))
757759
return false;
760+
offsetType = src1Opnd->GetType();
758761
}
759762

760763
IR::IntConstOpnd *src2Opnd = (instr->GetSrc2()->IsIntConstOpnd() ? instr->GetSrc2()->AsIntConstOpnd() : nullptr);
761-
if(src2Opnd && IntConstMath::Add(previousOffset, src2Opnd->GetValue(), &previousOffset))
764+
if(src2Opnd && IntConstMath::Add(previousOffset, src2Opnd->GetValue(), src2Opnd->GetType(), &previousOffset))
762765
return false;
763766

764767
// Given a bounds check (a <= b + offset), the offset may only be decreased such that it does not invalidate the invariant
@@ -768,22 +771,22 @@ bool IntBoundCheck::SetBoundOffset(const int offset, const bool isLoopCountBased
768771
return true;
769772

770773
IntConstType offsetDecrease;
771-
if(IntConstMath::Sub(previousOffset, offset, &offsetDecrease))
774+
if(IntConstMath::Sub(previousOffset, offset, offsetType, &offsetDecrease))
772775
return false;
773776

774777
Assert(offsetDecrease > 0);
775778
if(src1Opnd)
776779
{
777780
// Prefer to increase src1, as this is an upper bound check and src1 corresponds to the index
778781
IntConstType newSrc1Value;
779-
if(IntConstMath::Add(src1Opnd->GetValue(), offsetDecrease, &newSrc1Value))
782+
if(IntConstMath::Add(src1Opnd->GetValue(), offsetDecrease, src1Opnd->GetType(), &newSrc1Value))
780783
return false;
781784
src1Opnd->SetValue(newSrc1Value);
782785
}
783786
else if(dstOpnd)
784787
{
785788
IntConstType newDstValue;
786-
if(IntConstMath::Sub(dstOpnd->GetValue(), offsetDecrease, &newDstValue))
789+
if(IntConstMath::Sub(dstOpnd->GetValue(), offsetDecrease, dstOpnd->GetType(), &newDstValue))
787790
return false;
788791
if (newDstValue == 0)
789792
instr->FreeDst();

‎lib/Backend/IntConstMath.cpp‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft Corporation and contributors. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
#include "Backend.h"
7+
8+
bool IntConstMath::IsValid(IntConstType val, IRType type)
9+
{
10+
switch (type)
11+
{
12+
#if TARGET_32
13+
case TyInt32:
14+
case TyUint32:
15+
CompileAssert(sizeof(IntConstType) == sizeof(int32));
16+
return true;
17+
#elif TARGET_64
18+
case TyInt32:
19+
case TyUint32:
20+
return Math::FitsInDWord(val);
21+
case TyInt64:
22+
case TyUint64:
23+
CompileAssert(sizeof(IntConstType) == sizeof(int64));
24+
return true;
25+
#endif
26+
default:
27+
Assert(UNREACHED);
28+
return false;
29+
}
30+
}
31+
32+
bool IntConstMath::Add(IntConstType left, IntConstType right, IRType type, IntConstType * result)
33+
{
34+
bool overflowed = IntMathCommon<IntConstType>::Add(left, right, result);
35+
return overflowed || !IsValid(*result, type);
36+
}
37+
38+
bool IntConstMath::Sub(IntConstType left, IntConstType right, IRType type, IntConstType * result)
39+
{
40+
bool overflowed = IntMathCommon<IntConstType>::Sub(left, right, result);
41+
return overflowed || !IsValid(*result, type);
42+
}
43+
44+
bool IntConstMath::Mul(IntConstType left, IntConstType right, IRType type, IntConstType * result)
45+
{
46+
#if TARGET_32
47+
bool overflowed = Int32Math::Mul(left, right, result);
48+
CompileAssert(sizeof(IntConstType) == sizeof(int32));
49+
#elif TARGET_64
50+
bool overflowed = Int64Math::Mul(left, right, result);
51+
CompileAssert(sizeof(IntConstType) == sizeof(int64));
52+
#endif
53+
return overflowed || !IsValid(*result, type);
54+
}
55+
56+
bool IntConstMath::Div(IntConstType left, IntConstType right, IRType type, IntConstType * result)
57+
{
58+
bool overflowed = IntMathCommon<IntConstType>::Div(left, right, result);
59+
return overflowed || !IsValid(*result, type);
60+
}
61+
62+
bool IntConstMath::Mod(IntConstType left, IntConstType right, IRType type, IntConstType * result)
63+
{
64+
bool overflowed = IntMathCommon<IntConstType>::Mod(left, right, result);
65+
return overflowed || !IsValid(*result, type);
66+
}
67+
68+
bool IntConstMath::Dec(IntConstType val, IRType type, IntConstType * result)
69+
{
70+
bool overflowed = IntMathCommon<IntConstType>::Dec(val, result);
71+
return overflowed || !IsValid(*result, type);
72+
}
73+
74+
bool IntConstMath::Inc(IntConstType val, IRType type, IntConstType * result)
75+
{
76+
bool overflowed = IntMathCommon<IntConstType>::Inc(val, result);
77+
return overflowed || !IsValid(*result, type);
78+
}
79+
80+
bool IntConstMath::Neg(IntConstType val, IRType type, IntConstType * result)
81+
{
82+
bool overflowed = IntMathCommon<IntConstType>::Neg(val, result);
83+
return overflowed || !IsValid(*result, type);
84+
}

0 commit comments

Comments
 (0)