Security: X64 assembler incorrectly encodes RIP+disp operand when followed by immediate. |
|||||||||||
Issue descriptionI do not know if this is a security bug, but I'm erring on the side of caution. The encoding of RIP+disp32 operands in the X64 assembler is assuming that the operand is at the end of the instruction. That is, the computed offset is relative to the end of the *operand*, not the *instruction*. If the instruction contains a following immediate, like https://github.com/v8/v8/blob/master/src/x64/assembler-x64.cc#L3387 the address will be off-by-one (or more for larger immediates). It is very likely that this doesn't affect V8 at all, if it doesn't actually use RIP-relative addressing or doesn't use it with instructions with an immediate. If anything, it's a very detectable bug that would likely have been caught, but it's better to fix it before someone hits the bug :) The easiest fix is likely to add an optional third argument to emit_operand that tells it how many bytes will follow, defaulting to 0. (This was reported to me by ringgaard@ who is using the assembler on his own project)
,
Jun 20 2017
,
Jun 20 2017
,
Jun 20 2017
Deepti, several of these simd opcodes (cmpps for instance), seem to never have been used. I look to have gotten the immediate order wrong for them, so there's concern it's a security gotcha. Can we either: 1. Fix if we plan to use them 2. Drop if we don't. Thanks!
,
Jun 20 2017
I'm assigning low severity as it looks like a security issue, but not a dangerous one. Please feel free to re-adjust.
,
Jun 20 2017
This should not affect V8 as the code generator does not actually generate these instructions yet. These have been added as assembler support for upcoming features (SIMD in WebAssembly), and are not hooked up to anything, does that still make it a security issue? Regarding fixing these - I think it's best to fix them as these will be used pretty soon. As mentioned in the original description these are easily detectable once they are hooked up to the code generator, I've fixed a few of these for the integer vector instructions - usually due to incorrect operands/modrm bytes. In this particular case, the emit_sse_operand function should not be used for instructions with immediates, and needs it's own function like pextrd, pextrw etc.
,
Jun 21 2017
If the instructions aren't generated yet, I think that makes this a non-security issue. But we should definitely fix the issue if it's planned to use them soon.
,
Jul 12 2017
,
Feb 14 2018
I would say Security_Impact-Stable based on "not hooked up to anything", but #6 is from 6 months ago. :) Any updates here? I this bug now live in production, or fixed, or...? Thanks!
,
Feb 14 2018
There's two different issues here - the first mentioned by the reporter regarding RIP relative addressing, the second that there might be bugs in unused instructions. For RIP relative addressing, I believe the pc is being updated correctly, and the immediate is being emitted at the correct offset. Specifically for unused SIMD instructions mentioned by Brad above, most of these instructions are now in use, and the assembler/disassembler bugs associated with them have been fixed. There are still some unused instructions with the potential of bugs, but I don't think this is the right issue to track them with. If there is a specific example/test case that doesn't work with RIP relative addressing, I'm happy to look into it - if not, I think this bug can be closed.
,
Apr 2 2018
As per my earlier comment, I'm going to go ahead and mark this fixed, please re-open if this is still an issue.
,
Apr 3 2018
,
Jul 10
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by elawrence@chromium.org
, Jun 13 2017