Bugzilla

One way this might conceivably happen is if `aNowTime` went backwards, i.e. if we get a series of calls like:

* Call #1 to Tick(): We hit the code that I quoted above and we call `mStartTime.emplace(aNowTime);`.
* Call #2 to Tick(): For $WEIRD_REASON, this call receives an *older* `aNowTime` than the previous call. This gets stored in `mMostRecentRefresh` and it is **less than the mStartTime that we previously emplaced**.
* Call #3 to Tick(): We just proceed through the function, copying the ~bogus `mMostRecentRefresh` into `previousRefresh`.  We end up producing a negative value in `prev` as a result, which we then convert to uint32_t and trip this UBSan error.


Normally we wouldn't expect Call#2 to proceed since nsRefreshDriver has an early return if it detects time going backwards, here:
```
  if ((aNowTime <= mMostRecentRefresh) && !mTestControllingRefreshes &&
      aIsExtraTick == IsExtraTick::No) {
    return;
```
https://searchfox.org/mozilla-central/rev/361f258f46af4b9c881be81d1291000827c15704/layout/base/nsRefreshDriver.cpp#2154

However, the extra conditions there are interesting. It looks like it's conceivable we could get into this situation if either of those conditions are true.

Given that this was detected during fuzzing, I'm especially suspicious of the `mTestControllingRefreshes` bool there. I think that gets set by the `nsIDOMWindowUtils::advanceTimeAndRefresh` privileged JS API.  Tyson, do you know if the fuzzer might've been using that nsIDOMWindowUtils API at all?

(If not, then it's also conceivable this could be associated with the `aIsExtraTick` thing, too.)

One way this might conceivably happen is if `aNowTime` went backwards, i.e. if we get a series of calls like:

* Call #1 to Tick(): We hit the code that I quoted above and we call `mStartTime.emplace(aNowTime)` for an entry in `mStartTable`.
* Call #2 to Tick(): For $WEIRD_REASON, this call receives an *older* `aNowTime` than the previous call. This gets stored in `mMostRecentRefresh` and it is **less than the mStartTime that we previously emplaced**.
* Call #3 to Tick(): We just proceed through the function, copying the ~bogus `mMostRecentRefresh` into `previousRefresh`.  We end up producing a negative value in `prev` as a result, which we then convert to uint32_t and trip this UBSan error.


Normally we wouldn't expect Call#2 to proceed since nsRefreshDriver has an early return if it detects time going backwards, here:
```
  if ((aNowTime <= mMostRecentRefresh) && !mTestControllingRefreshes &&
      aIsExtraTick == IsExtraTick::No) {
    return;
```
https://searchfox.org/mozilla-central/rev/361f258f46af4b9c881be81d1291000827c15704/layout/base/nsRefreshDriver.cpp#2154

However, the extra conditions there are interesting. It looks like it's conceivable we could get into this situation if either of those conditions are true.

Given that this was detected during fuzzing, I'm especially suspicious of the `mTestControllingRefreshes` bool there. I think that gets set by the `nsIDOMWindowUtils::advanceTimeAndRefresh` privileged JS API.  Tyson, do you know if the fuzzer might've been using that nsIDOMWindowUtils API at all?

(If not, then it's also conceivable this could be associated with the `aIsExtraTick` thing, too.)

One way this might conceivably happen is if `aNowTime` went backwards, i.e. if we get a series of calls like:

* Call #1 to Tick(): We hit the code that I quoted above and we call `mStartTime.emplace(aNowTime)` for an entry in `mStartTable`.
* Call #2 to Tick(): For $WEIRD_REASON, this second tick receives an *older* `aNowTime` than the previous call. This gets stored in `mMostRecentRefresh`.  Notably, this puts us into a situation where `mMostRecentRefresh` is **less than a `mStartTime` that we previously emplaced in `mStartTable`**.
* Call #3 to Tick(): We just proceed through the function, copying the ~bogus `mMostRecentRefresh` into `previousRefresh`.  We end up producing a negative value in `prev` as a result, which we then convert to uint32_t and trip this UBSan error.


Normally we wouldn't expect Call#2 to proceed since nsRefreshDriver has an early return if it detects time going backwards, here:
```
  if ((aNowTime <= mMostRecentRefresh) && !mTestControllingRefreshes &&
      aIsExtraTick == IsExtraTick::No) {
    return;
```
https://searchfox.org/mozilla-central/rev/361f258f46af4b9c881be81d1291000827c15704/layout/base/nsRefreshDriver.cpp#2154

However, the extra conditions there are interesting. It looks like it's conceivable we could get into this situation if either of those conditions are true.

Given that this was detected during fuzzing, I'm especially suspicious of the `mTestControllingRefreshes` bool there. I think that gets set by the `nsIDOMWindowUtils::advanceTimeAndRefresh` privileged JS API.  Tyson, do you know if the fuzzer might've been using that nsIDOMWindowUtils API at all?

(If not, then it's also conceivable this could be associated with the `aIsExtraTick` thing, too.)

One way this might conceivably happen is if `aNowTime` went backwards, i.e. if we get a series of calls like:

* Call #1 to Tick(): We hit the code that I quoted above and we call `mStartTime.emplace(aNowTime)` for an entry in `mStartTable`.
* Call #2 to Tick(): For $WEIRD_REASON, this second tick receives an *older* `aNowTime` than the previous call. This gets stored in `mMostRecentRefresh`.  Notably, this puts us into a situation where `mMostRecentRefresh` is **less than a `mStartTime` that we previously emplaced in `mStartTable`**.
* Call #3 to Tick(): We just proceed through the function, copying the ~bogus `mMostRecentRefresh` into `previousRefresh`.  We end up producing a negative value in `prev` when we subtract `mStartTime` from it, which we then convert to uint32_t and trip this UBSan error.


Normally we wouldn't expect Call#2 to proceed since nsRefreshDriver has an early return if it detects time going backwards, here:
```
  if ((aNowTime <= mMostRecentRefresh) && !mTestControllingRefreshes &&
      aIsExtraTick == IsExtraTick::No) {
    return;
```
https://searchfox.org/mozilla-central/rev/361f258f46af4b9c881be81d1291000827c15704/layout/base/nsRefreshDriver.cpp#2154

However, the extra conditions there are interesting. It looks like it's conceivable we could get into this situation if either of those conditions are true.

Given that this was detected during fuzzing, I'm especially suspicious of the `mTestControllingRefreshes` bool there. I think that gets set by the `nsIDOMWindowUtils::advanceTimeAndRefresh` privileged JS API.  Tyson, do you know if the fuzzer might've been using that nsIDOMWindowUtils API at all?

(If not, then it's also conceivable this could be associated with the `aIsExtraTick` thing, too.)