Skip to content

Commit

Permalink
Fix heap use after free in MoveTool (#1791)
Browse files Browse the repository at this point in the history 
Also gets rid of a layer pointer in SelectTool
  • Loading branch information
@J5lx
J5lx authored Oct 17, 2023
1 parent 6a4fe12 commit 127cd62
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 54 deletions.
30 changes: 9 additions & 21 deletions core_lib/src/tool/movetool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ void MoveTool::updateSettings(const SETTING setting)

void MoveTool::pointerPressEvent(PointerEvent* event)
{
mCurrentLayer = currentPaintableLayer();
if (mCurrentLayer == nullptr) return;
Layer* currentLayer = currentPaintableLayer();
if (currentLayer == nullptr) return;

if (mEditor->select()->somethingSelected())
{
beginInteraction(event->modifiers(), mCurrentLayer);
beginInteraction(event->modifiers(), currentLayer);
}
if (mEditor->overlays()->anyOverlayEnabled())
{
Expand All @@ -124,8 +124,8 @@ void MoveTool::pointerPressEvent(PointerEvent* event)

void MoveTool::pointerMoveEvent(PointerEvent* event)
{
mCurrentLayer = currentPaintableLayer();
if (mCurrentLayer == nullptr) return;
Layer* currentLayer = currentPaintableLayer();
if (currentLayer == nullptr) return;

if (mScribbleArea->isPointerInUse()) // the user is also pressing the mouse (dragging)
{
Expand All @@ -149,9 +149,9 @@ void MoveTool::pointerMoveEvent(PointerEvent* event)
mEditor->select()->setMoveModeForAnchorInRange(getCurrentPoint());
mScribbleArea->updateToolCursor();

if (mCurrentLayer->type() == Layer::VECTOR)
if (currentLayer->type() == Layer::VECTOR)
{
storeClosestVectorCurve(mCurrentLayer);
storeClosestVectorCurve(currentLayer);
}
}
mEditor->updateFrame();
Expand Down Expand Up @@ -305,12 +305,6 @@ void MoveTool::storeClosestVectorCurve(Layer* layer)
selectMan->setCurves(pVecImg->getCurvesCloseTo(getCurrentPoint(), selectMan->selectionTolerance()));
}

void MoveTool::cancelChanges()
{
mScribbleArea->cancelTransformedSelection();
mEditor->deselectAll();
}

void MoveTool::applyTransformation()
{
SelectionManager* selectMan = mEditor->select();
Expand All @@ -325,14 +319,9 @@ void MoveTool::applyTransformation()

bool MoveTool::leavingThisTool()
{
if (mCurrentLayer)
if (currentPaintableLayer())
{
switch (mCurrentLayer->type())
{
case Layer::BITMAP: applyTransformation(); break;
case Layer::VECTOR: applyTransformation(); break;
default: break;
}
applyTransformation();
}
return true;
}
Expand Down Expand Up @@ -406,7 +395,6 @@ QCursor MoveTool::cursor(MoveMode mode) const
}
default:
return Qt::ArrowCursor;
break;
}
cursorPainter.end();

Expand Down
2 changes: 0 additions & 2 deletions core_lib/src/tool/movetool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class MoveTool : public BaseTool
void setShowSelectionInfo(const bool b) override;

private:
void cancelChanges();
void applyTransformation();
void updateSettings(const SETTING setting);

Expand All @@ -60,7 +59,6 @@ class MoveTool : public BaseTool

Layer* currentPaintableLayer();

Layer* mCurrentLayer = nullptr;
qreal mRotatedAngle = 0.0;
qreal mPreviousAngle = 0.0;
int mRotationIncrement = 0;
Expand Down
2 changes: 0 additions & 2 deletions core_lib/src/tool/penciltool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ class PencilTool : public StrokeTool
void setUseFillContour(const bool useFillContour) override;

private:
QColor mCurrentPressuredColor{ 0, 0, 0, 255 };
QPointF mLastBrushPoint{ 0, 0 };
qreal mOpacity = 1.0f;
QPointF mMouseDownPoint;
};

Expand Down
44 changes: 19 additions & 25 deletions core_lib/src/tool/selecttool.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,15 @@ void SelectTool::setShowSelectionInfo(const bool b)
settings.setValue("ShowSelectionInfo", b);
}

void SelectTool::beginSelection()
void SelectTool::beginSelection(Layer* currentLayer)
{
auto selectMan = mEditor->select();

if (selectMan->somethingSelected() && mMoveMode != MoveMode::NONE) // there is something selected
{
if (mCurrentLayer->type() == Layer::VECTOR)
if (currentLayer->type() == Layer::VECTOR)
{
VectorImage* vectorImage = static_cast<LayerVector*>(mCurrentLayer)->getLastVectorImageAtFrame(mEditor->currentFrame(), 0);
VectorImage* vectorImage = static_cast<LayerVector*>(currentLayer)->getLastVectorImageAtFrame(mEditor->currentFrame(), 0);
if (vectorImage != nullptr) {
vectorImage->deselectAll();
}
Expand All @@ -120,24 +120,24 @@ void SelectTool::beginSelection()

void SelectTool::pointerPressEvent(PointerEvent* event)
{
mCurrentLayer = mEditor->layers()->currentLayer();
if (mCurrentLayer == nullptr) return;
if (!mCurrentLayer->isPaintable()) { return; }
Layer* currentLayer = mEditor->layers()->currentLayer();
if (currentLayer == nullptr) return;
if (!currentLayer->isPaintable()) { return; }
if (event->button() != Qt::LeftButton) { return; }
auto selectMan = mEditor->select();

selectMan->setMoveModeForAnchorInRange(getCurrentPoint());
mMoveMode = selectMan->getMoveMode();
mStartMoveMode = mMoveMode;

beginSelection();
beginSelection(currentLayer);
}

void SelectTool::pointerMoveEvent(PointerEvent*)
{
mCurrentLayer = mEditor->layers()->currentLayer();
if (mCurrentLayer == nullptr) { return; }
if (!mCurrentLayer->isPaintable()) { return; }
Layer* currentLayer = mEditor->layers()->currentLayer();
if (currentLayer == nullptr) { return; }
if (!currentLayer->isPaintable()) { return; }
auto selectMan = mEditor->select();

if (!selectMan->somethingSelected()) { return; }
Expand All @@ -150,9 +150,9 @@ void SelectTool::pointerMoveEvent(PointerEvent*)
{
controlOffsetOrigin(getCurrentPoint(), mAnchorOriginPoint);

if (mCurrentLayer->type() == Layer::VECTOR)
if (currentLayer->type() == Layer::VECTOR)
{
VectorImage* vectorImage = static_cast<LayerVector*>(mCurrentLayer)->getLastVectorImageAtFrame(mEditor->currentFrame(), 0);
VectorImage* vectorImage = static_cast<LayerVector*>(currentLayer)->getLastVectorImageAtFrame(mEditor->currentFrame(), 0);
if (vectorImage != nullptr) {
vectorImage->select(selectMan->mapToSelection(QPolygonF(selectMan->mySelectionRect())).boundingRect());
}
Expand All @@ -164,8 +164,8 @@ void SelectTool::pointerMoveEvent(PointerEvent*)

void SelectTool::pointerReleaseEvent(PointerEvent* event)
{
mCurrentLayer = mEditor->layers()->currentLayer();
if (mCurrentLayer == nullptr) return;
Layer* currentLayer = mEditor->layers()->currentLayer();
if (currentLayer == nullptr) return;
if (event->button() != Qt::LeftButton) return;

// if there's a small very small distance between current and last point
Expand All @@ -181,7 +181,7 @@ void SelectTool::pointerReleaseEvent(PointerEvent* event)
}
else
{
keepSelection();
keepSelection(currentLayer);
}

mStartMoveMode = MoveMode::NONE;
Expand All @@ -200,25 +200,19 @@ bool SelectTool::maybeDeselect()
* @brief SelectTool::keepSelection
* Keep selection rect and normalize if invalid
*/
void SelectTool::keepSelection()
void SelectTool::keepSelection(Layer* currentLayer)
{
auto selectMan = mEditor->select();
if (mCurrentLayer->type() == Layer::VECTOR)
if (currentLayer->type() == Layer::VECTOR)
{
VectorImage* vectorImage = static_cast<LayerVector*>(mCurrentLayer)->getLastVectorImageAtFrame(mEditor->currentFrame(), 0);
VectorImage* vectorImage = static_cast<LayerVector*>(currentLayer)->getLastVectorImageAtFrame(mEditor->currentFrame(), 0);
if (vectorImage == nullptr) { return; }
selectMan->setSelection(vectorImage->getSelectionRect(), false);
}
}

void SelectTool::controlOffsetOrigin(QPointF currentPoint, QPointF anchorPoint)
{
QPointF offset = offsetFromPressPos();

if (editor()->layers()->currentLayer()->type() == Layer::BITMAP) {
offset = QPointF(offset).toPoint();
}

// when the selection is none, manage the selection Origin
if (mStartMoveMode != MoveMode::NONE) {
QRectF rect = mSelectionRect;
Expand Down Expand Up @@ -302,7 +296,7 @@ bool SelectTool::keyPressEvent(QKeyEvent* event)
break;
}

// Follow the generic behaviour anyway
// Follow the generic behavior anyway
return BaseTool::keyPressEvent(event);
}

Expand Down
7 changes: 3 additions & 4 deletions core_lib/src/tool/selecttool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,20 @@ class SelectTool : public BaseTool
void manageSelectionOrigin(QPointF currentPoint, QPointF originPoint);
void controlOffsetOrigin(QPointF currentPoint, QPointF anchorPoint);

void beginSelection();
void keepSelection();
void beginSelection(Layer* currentLayer);
void keepSelection(Layer* currentLayer);

QPointF offsetFromPressPos();

inline bool isSelectionPointValid() { return mAnchorOriginPoint != getLastPoint(); }
bool maybeDeselect();

// Store selection origin so we can calculate
// Store selection origin, so we can calculate
// the selection rectangle in mousePressEvent.
QPointF mAnchorOriginPoint;
MoveMode mMoveMode;
MoveMode mStartMoveMode = MoveMode::NONE;
QRectF mSelectionRect;
Layer* mCurrentLayer = nullptr;

QPixmap mCursorPixmap = QPixmap(24, 24);
};
Expand Down

0 comments on commit 127cd62

Please sign in to comment.