Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

17-11 Security Update #4226

Merged
merged 18 commits into from
Nov 16, 2017
Merged

17-11 Security Update #4226

merged 18 commits into from
Nov 16, 2017

Conversation

leirocks
Copy link
Contributor

@leirocks leirocks commented Nov 14, 2017

leirocks and others added 6 commits November 12, 2017 14:11
…:VEval function - Individual

do not use eval map if we are eval-ing PropertyString
…o RCE - Zero Day Initiative

Fix *::TransferPass0 overflowing the returning integer, the allocation part is the only point that using the returning value.
…ontrolled allocation size and write - Individual

When parsing a string template literal, we temporarily suspend updating some of the offset counters. If an attacker crafts a string template that contains multibyte characters, it is possible to overflow the minLine counter. The attacker can directly control the size of the overflow, which is then used in an allocation and offset calculation when handling a parse error. Further control can be gained by triggering a scanner capture and restore to propagate this bad counter value.
Unfortunately this would have been caught by existing bounds checks, but they are asserts only. There are other instances of these around the codebase, but I did not see any exploits for those.
To fix this, we manually update the minLine counter at the point where it would normally have been updated outside of a string template. I also changed the subtraction overflow asserts to be failfasts and added an assert for detecting overflow with multibyte characters. In the non-assert case, we will return 0. Other similar checks in IchMinTok and IchLimTok have also been converted.
Copy link
Contributor

@MSLaguana MSLaguana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version number change looks good.

@leirocks
Copy link
Contributor Author

fixed "BackEnd.h" with "Backend.h"

Copy link
Contributor

@rajatd rajatd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@@ -4772,64 +4772,45 @@ namespace Js
return returnValue;
}

template<typename T, typename T(*func)(Var, ScriptContext*)> bool MemsetConversion(Var value, ScriptContext* scriptContext, T* result)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang is complaining about this line, it doesn't seem to like the typename T(*func)(Var, ScriptContext*)

�[1m/mnt/j/w/Microsoft_ChakraCore/release_1.7/shared_ubuntu_linux_debug_prtest/lib/Runtime/Language/JavascriptOperators.cpp:4775:35: �[0m�[0;1;31merror: �[0m�[1mexpected a qualified name after 'typename' [-Werror]�[0m
12:35:58     template<typename T, typename T(*func)(Var, ScriptContext*)> bool MemsetConversion(Var value, ScriptContext* scriptContext, T* result)

Copy link
Contributor

@akroshg akroshg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@MSLaguana
Copy link
Contributor

@dotnet-bot test OSX static_osx_osx_debug please

Cellule and others added 10 commits November 14, 2017 15:09
… 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.
… Qihoo 360

OOM in the Array.Shift method have left the array in the bad state and later it got overlapped and exploited. Fixed that by making the any exception as failfast in that region
…vidual

Export was not taking care of destructuring nodes, leading to type confusion. Fixed that by adding support for walking those nodes.
…en for function objects with inline cache

When trying to get inlineCaches for a ScriptFunctionWithInlineCache if the function got redefered and didn't got reparsed function body won’t be there (hence no inlineCache on function body). Check that and reset the inlineCache of ScriptFunctionWithInlineCache for such case.
@leirocks
Copy link
Contributor Author

fix build break in clang:
template<typename T, typename T(func)(Var, ScriptContext)>
==> template<typename T, T(func)(Var, ScriptContext)>

@leirocks
Copy link
Contributor Author

@dotnet-bot test OSX static_osx_osx_debug please

@leirocks
Copy link
Contributor Author

@dotnet-bot test Ubuntu shared_ubuntu_linux_debug please

@MSLaguana
Copy link
Contributor

Looks like the test timeout failure isn't just OSX, it's also happening on linux debug builds:

18:33:26 
ERROR: Test timed out!
18:33:26 /mnt/j/w/Microsoft_ChakraCore/release_1.7/static_ubuntu_linux_debug_prtest/out/Debug/ch -WERExceptionSupport -ExtendedErrorStackForTestHost -BaselineMode -forceNative -off:simpleJit -bgJitDelay:0 -dynamicprofileinput:profile.dpl.793 -off:deferparse /mnt/j/w/Microsoft_ChakraCore/release_1.7/static_ubuntu_linux_debug_prtest/test/Function/FuncBody.bug232281.js

… eval on property will be slower, especially in debug build.

here empty string or single char string is backed by property string (an optimazation for using inline buffer), and these two tests
keep eval on empty string until stack overflow, which causes timeout on debug build.

changing to eval on two spaces instead of empty string to avoid the issue
@chakrabot chakrabot merged commit 1be2d26 into chakra-core:release/1.7 Nov 16, 2017
chakrabot pushed a commit that referenced this pull request Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants