Bugzilla

Sorry to place this under the bug bounty section I wasn't sure how to file a security report.

I discovered the following potential vulnerability by code inspection. I wanted to report it as soon as possible to get the opinions of Mozilla developers and security team to see if this is worth investigating.

In the file /layout/base/presshell.cpp the `function bool PresShell::ScrollFrameRectIntoView(nsIFrame* aFrame, const nsRect& aRect, nsIPresShell::ScrollAxis aVertical, nsIPresShell::ScrollAxis aHorizontal, uint32_t aFlags) `


gets two raw pointers:

```cpp
  bool didScroll = false;
  // This function needs to work even if rect has a width or height of 0.
  nsRect rect = aRect;
  nsIFrame* container = aFrame; //raw pointer to a frame
  // Walk up the frame hierarchy scrolling the rect into view and
  // keeping rect relative to container
  do {
    nsIScrollableFrame* sf = do_QueryFrame(container); //raw pointer to scrollable frame
    if (sf) {
      nsPoint oldPosition = sf->GetScrollPosition();
      nsRect targetRect = rect;
```

...SNIP...

Later on the function calls `ScrollToShowRect(sf, targetRect - sf->GetScrolledFrame()->GetPosition(),
                       aVertical, aHorizontal, aFlags);`
ScrollToShowRect calls 
`aFrameAsScrollable->ScrollTo(scrollPt, scrollMode, &allowedRange);`

The header file nsIScrollableFrame.h /layout/generic/nsIScrollableFrame.h has the following comment and function definition:
```cpp
/**
   * @note This method might destroy the frame, pres shell and other objects.
   * Clamps aScrollPosition to GetScrollRange and sets the scroll position
   * to that value.
   * @param aRange If non-null, specifies area which contains aScrollPosition
   * and can be used for choosing a performance-optimized scroll position.
   * Any point within this area can be chosen.
   * The choosen point will be as close as possible to aScrollPosition.
   */
  virtual void ScrollTo(nsPoint aScrollPosition, ScrollMode aMode,
                        const nsRect* aRange = nullptr,
                        nsIScrollbarMediator::ScrollSnapMode aSnap =
                            nsIScrollbarMediator::DISABLE_SNAP) = 0;
```


The comment says that "this method might destroy the frame, presshell and other objects." 

Back in our original function the raw pointers are used after the call to ScrollToShowRect
...SNIP...
```
      nsPoint newPosition = sf->LastScrollDestination();
```
...SNIP...
```
    if (container->IsTransformed()) { //
```

So basically the full snippet is this:

```cpp
bool PresShell::ScrollFrameRectIntoView(nsIFrame* aFrame, const nsRect& aRect,
                                        nsIPresShell::ScrollAxis aVertical,
                                        nsIPresShell::ScrollAxis aHorizontal,
                                        uint32_t aFlags) {
  bool didScroll = false;
  // This function needs to work even if rect has a width or height of 0.
  nsRect rect = aRect;

  nsIFrame* container = aFrame; //raw pointer here
  // Walk up the frame hierarchy scrolling the rect into view and
  // keeping rect relative to container
  do {
    nsIScrollableFrame* sf = do_QueryFrame(container); //raw pointer here
    if (sf) {
      nsPoint oldPosition = sf->GetScrollPosition();
      nsRect targetRect = rect;
      // Inflate the scrolled rect by the container's padding in each dimension,
      // unless we have 'overflow-clip-box-*: content-box' in that dimension.
      auto* disp = container->StyleDisplay();
      if (disp->mOverflowClipBoxBlock == StyleOverflowClipBox::ContentBox ||
          disp->mOverflowClipBoxInline == StyleOverflowClipBox::ContentBox) {
        WritingMode wm = container->GetWritingMode();
        bool cbH = (wm.IsVertical() ? disp->mOverflowClipBoxBlock
                                    : disp->mOverflowClipBoxInline) ==
                   StyleOverflowClipBox::ContentBox;
        bool cbV = (wm.IsVertical() ? disp->mOverflowClipBoxInline
                                    : disp->mOverflowClipBoxBlock) ==
                   StyleOverflowClipBox::ContentBox;
        nsMargin padding = container->GetUsedPadding();
        if (!cbH) {
          padding.left = padding.right = nscoord(0);
        }
        if (!cbV) {
          padding.top = padding.bottom = nscoord(0);
        }
        targetRect.Inflate(padding);
      }
      ScrollToShowRect(sf, targetRect - sf->GetScrolledFrame()->GetPosition(),
                       aVertical, aHorizontal, aFlags);       //Frames and other objects can be deleted Maybe a UAF with the sf argument. 
      nsPoint newPosition = sf->LastScrollDestination(); //Potential UAF
      // If the scroll position increased, that means our content moved up,
      // so our rect's offset should decrease
      rect += oldPosition - newPosition;

      if (oldPosition != newPosition) {
        didScroll = true;
      }

      // only scroll one container when this flag is set
      if (aFlags & nsIPresShell::SCROLL_FIRST_ANCESTOR_ONLY) {
        break;
      }
    }
    nsIFrame* parent;
    if (container->IsTransformed()) //Potential UAF{
      container->GetTransformMatrix(nullptr, &parent);
```


I haven't tried to exploit this yet but I think there are a few entry points that would lead to the vulnerable call. I think anywhere the SCROLL_SYNCHRONOUS flag passed into a ScrollSelectionIntoView function would be potential vulnerable call.

```cpp
  /**
   * ScrollSelectionIntoView scrolls a region of the selection,
   * so that it is visible in the scrolled view.
   *
   * @param aSelectionType the selection to scroll into view.
   *
   * @param aRegion the region inside the selection to scroll into view.
   *
   * @param aFlags the scroll flags.  Valid bits include:
   *   * SCROLL_SYNCHRONOUS: when set, scrolls the selection into view 
   *     before returning. If not set, posts a request which is processed
   *     at some point after the method returns.
   *   * SCROLL_FIRST_ANCESTOR_ONLY: if set, only the first ancestor will be
   *     scrolled into view.
   */
  /*unsafe*/
  nsresult ScrollSelectionIntoView(mozilla::SelectionType aSelectionType,
                                   SelectionRegion aRegion,
                                   int16_t aFlags) const;
```

Does anyone have any tips on how I could debug this or verify the issue?