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

Changes addressing CVE_2016-3382, CVE-2016-3386, CVE-2016-3389, CVE-2016-3390, CVE-2016-7189, and a mitigation of a CFG bypass. #1737

Merged
merged 1 commit into from
Oct 13, 2016

Conversation

pleath
Copy link
Contributor

@pleath pleath commented Oct 13, 2016

Details:

Calls that target the external thunks should decrement the callinfo param count if the extra-param flag is set.

Don't optimize spread operation in a parameter list if the array we're spreading may have gaps. Accessing an element in the prototype chain may have side-effects that invalidate the optimization.

Port disabling of UT that times out

Type confusion in JavascriptArray
Type confusion in JavascriptArray::TemplatedGetItem()
Array.prototype.join()
Array.prototype.indexOf()
Array.prototype.lastIndexOf()

Type confusion in JavascriptArray::TemplatedGetItem()
Function.prototype.apply()

Type confusion in JavascriptArray::MapHelper()
Array.prototype.map()

CRC computation and validation for the encoder buffer

Premise:
Encoder phase takes longer time for a relatively larger function. So the buffer to which we write the encoded bytes will be RWX all the while till it completes the encoding.
This time is big enough for the main thread to write in this region.
We then transfer the data to the final buffer and execute the code in the buffer(which, now, also contains the modified code).

Mitigation:
We can check the integrity of the buffer data using CRC32(Cyclic Redundancy Check) at suitable spots.
Following is the mechanism for validation:

  • Start with a random CRC seed.
  • Compute the CRC1 during the encoding phase.
  • Validate the CRC1 during branch shortening.
  • Compute CRC2 during branch shortening.
  • Validate CRC2 (or CRC1, if branch shortening didn't happen) after copying the entire buffer to the final RX buffer.
  • Finally, register the entry point as a valid CFG target.

CRC32 Intrinsic instruction is available only on SSE4 and above. Hence for other cases, CRC32 algorithm is implemented.
We were storing LabelInstr* directly in the encoded bytes - Moved it to be stored in a property. - To enable CRC calculation.

Perf results:
No visible changes in console benchmark run (desktop and low-memory device).

Fixes to use-after-free in Globopt, Lowering.

Tail duplication consists of the following code :

branchEntry->ReplaceTarget(mergeLabel, tailBranch->GetTarget());
instr = branchEntry;

branchEntry is a reference to a SList node that can get deleted within
ReplaceTarget function. Subsequent use of the same reference is referring
to a freed value. Fix by caching branchEntry before ReplaceTarget.

Lowering floor builtin code creates a 'zero' MemRefOpnd, which gets passed
through Legalizer, which can delete the Opnd. Subsequent uses of the
MemRefOpnd in Lowering refers to a freed value. This is fixed by
AutoReuseOpnd which will avoid this scenario.

Correcting the version check for SSE4

…016-3389,

CVE-2016-3390, CVE-2016-7189, and a mitigation of a CFG bypass.

Details:

Calls that target the external thunks should decrement the callinfo param count if the extra-param flag is set.

Don't optimize spread operation in a parameter list if the array we're spreading may have gaps. Accessing an element in the prototype chain may have side-effects that invalidate the optimization.

Port disabling of UT that times out

Type confusion in JavascriptArray
Type confusion in JavascriptArray::TemplatedGetItem()
    Array.prototype.join()
    Array.prototype.indexOf()
    Array.prototype.lastIndexOf()

Type confusion in JavascriptArray::TemplatedGetItem()
    Function.prototype.apply()

Type confusion in JavascriptArray::MapHelper()
    Array.prototype.map()

CRC computation and validation for the encoder buffer

Premise:
Encoder phase takes longer time for a relatively larger function. So the buffer to which we write the encoded bytes will be RWX all the while till it completes the encoding.
This time is big enough for the main thread to write in this region.
We then transfer the data to the final buffer and execute the code in the buffer(which, now, also contains the modified code).

Mitigation:
We can check the integrity of the buffer data using CRC32(Cyclic Redundancy Check) at suitable spots.
Following is the mechanism for validation:
- Start with a random CRC seed.
- Compute the CRC1 during the encoding phase.
- Validate the CRC1 during branch shortening.
- Compute CRC2 during branch shortening.
- Validate CRC2 (or CRC1, if branch shortening didn't happen) after copying the entire buffer to the final RX buffer.
- Finally, register the entry point as a valid CFG target.

CRC32 Intrinsic instruction is available only on SSE4 and above. Hence for other cases, CRC32 algorithm is implemented.
We were storing LabelInstr* directly in the encoded bytes - Moved it to be stored in a property. - To enable CRC calculation.

Perf results:
No visible changes in console benchmark run (desktop and low-memory device).

Fixes to use-after-free in Globopt, Lowering.

Tail duplication consists of the following code :

 branchEntry->ReplaceTarget(mergeLabel, tailBranch->GetTarget());
 instr = branchEntry;

branchEntry is a reference to a SList node that can get deleted within
ReplaceTarget function. Subsequent use of the same reference is referring
to a freed value. Fix by caching branchEntry before ReplaceTarget.

Lowering floor builtin code creates a 'zero' MemRefOpnd, which gets passed
through Legalizer, which can delete the Opnd. Subsequent uses of the
MemRefOpnd in Lowering refers to a freed value. This is fixed by
AutoReuseOpnd which will avoid this scenario.

Correcting the version check for SSE4
@pleath
Copy link
Contributor Author

pleath commented Oct 13, 2016

@meg-gupta , @satheeshravi , @suwc , please review your individual changes. (Thanks.)

@pleath pleath changed the title Changes addressing CVE_2016-3382, CVE-2016-3385, CVE-2016-3386, CVE-2016-3389, CVE-2016-3390, CVE-2016-7189, and a mitigation of a CFG bypass. Changes addressing CVE_2016-3382, CVE-2016-3386, CVE-2016-3389, CVE-2016-3390, CVE-2016-7189, and a mitigation of a CFG bypass. Oct 13, 2016
@satheeshravi
Copy link
Contributor

LGTM

1 similar comment
@meg-gupta
Copy link
Contributor

LGTM

@suwc
Copy link

suwc commented Oct 13, 2016

:shipit:

@suwc
Copy link

suwc commented Oct 13, 2016

LGTM

@chakrabot chakrabot merged commit f05c42e into chakra-core:release/1.2 Oct 13, 2016
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.

6 participants