> > > That being said, sometimes such nodes are missing, which can lead to > security bugs. That's why there is lots of code checking for zero > extension, see e.g. > https://source.chromium.org/chromium/chromium/src/+/main:v8/src/compiler/backend/x64/code-generator-x64.cc;l=1898;drc=6bd44dfe57d8d9673c34eda1ae27d66a7f5ec637 > . > > Is keep uint32 values "sign-extended"(extend bit 31 into bits 63 through > 32) or `ChangeUint32ToUint64` without zero-extension that may lead to > security bugs? If it's the former, could you give more details? >
Probably both, though the former seems harder to trigger (as uint32 values with the top bit 1 are probably rare, and if the top bit is 0 anyway then "sign-extension" of course does the same as zero-extension). Wasm memories and/or TypedArrays with >2GB size might be ways to flush out issues. > > I think we zero-extend most 32-bit values in V8, however there are a few > cases where we must explicitly sign-extend: specifically when a (signed) > int32 will be added to a 64-bit value, such as for address+offset > computations. > > We do rely on 32-bit values being zero-extended in some places, so it's > important to keep those tests working. > > Will 32-bit values be added to 64-bit values without promotion to 64-bit? > I think if the operations are all between 32-bit values, then the > "sign-exntension" won't cause problems? > Besides, I thought only MIPS64 and RV64 care about this issue, X64 and > ARM64 have full 32-bit registers support, why do they still rely on the > zero-extension? > We commonly add 32-bit offsets to 64-bit object start addresses. There *should* be an explicit 64-bit promotion in every case, though we've seen bugs where that was missing. Since x86 hardware interprets offsets as signed values, doing a sign-extension of a value that's meant to be a uint (and hence positive) would change the result of the computation. Since we're talking about memory addresses, incorrect computations are crashes at best and security bugs at worst. (In fact, on x86 we've (also?) encountered the opposite issue: negative offsets do show up in some cases (e.g. HeapObject::kMapOffset, or when iterating/indexing backwards for some reason), and those have to be sign-extended.) My key takeaways are: (1) zero- vs. sign-extension matters, especially when 32-bit offsets and 64-bit base addresses are concerned (2) it's hard to get this right, because there are so many code paths where some variation of this happens. So it makes sense to be super diligent, e.g. by adding extra checks (behind #ifdef DEBUG and/or FLAG_debug_code) and fuzzer tests. -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAKSzg3RMCohrDr7A7pOOMVeEmwzcnSHLT45hpuiYyi70vvPwmA%40mail.gmail.com.
