1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid use after free in tst_qsequentialanimationgroup

Browse Source 
The test connects finished to the groups clear method, which in turn
deletes the animation instance. Thus, no member must be accessed after
calling stop, unless we use a (costly) QPointer to guard against
deletion.
Notify earlier that totalCurrentTime changed to avoid the issue.
As a drive-by, modernize the connect in the test.

Fixes: QTBUG-94143
Change-Id: I923101107b7f79115be69a58c8e8d5177a98d48f
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
Reviewed-by: Sona Kurazyan <sona.kurazyan@qt.io>
This commit is contained in:
Fabian Kosmale 2021-06-02 08:50:35 +02:00
parent 78ed8034d2
commit d5ab0101ff
2 changed files with 7 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/corelib/animation/qabstractanimation.cpp
View File

@ -1346,6 +1346,12 @@ void QAbstractAnimation::setCurrentTime(int msecs)
if (d->currentLoop != oldLoop) if (d->currentLoop != oldLoop)
d->currentLoop.notify(); d->currentLoop.notify();
/* Notify before calling stop: As seen in tst_QSequentialAnimationGroup::clear
* we might delete the animation when stop is called. Thus after stop no member
* of the object must be used anymore.
*/
if (oldCurrentTime != d->totalCurrentTime)
d->totalCurrentTime.notify();
// All animations are responsible for stopping the animation when their // All animations are responsible for stopping the animation when their
// own end state is reached; in this case the animation is time driven, // own end state is reached; in this case the animation is time driven,
// and has reached the end. // and has reached the end.
@ -1353,8 +1359,6 @@ void QAbstractAnimation::setCurrentTime(int msecs)
|| (d->direction == Backward && d->totalCurrentTime == 0)) { || (d->direction == Backward && d->totalCurrentTime == 0)) {
stop(); stop();
} }
if (oldCurrentTime != d->totalCurrentTime)
d->totalCurrentTime.notify();
} }
/*! /*!

2
tests/auto/corelib/animation/qsequentialanimationgroup/tst_qsequentialanimationgroup.cpp
View File

@ -1614,7 +1614,7 @@ void tst_QSequentialAnimationGroup::clear()
{ {
SequentialAnimationGroup group; SequentialAnimationGroup group;
QPointer<QAbstractAnimation> anim1 = new DummyPropertyAnimation(&group); QPointer<QAbstractAnimation> anim1 = new DummyPropertyAnimation(&group);
group.connect(anim1, SIGNAL(finished()), SLOT(clear())); connect(anim1, &QAbstractAnimation::finished, &group, &QSequentialAnimationGroup::clear);
new DummyPropertyAnimation(&group); new DummyPropertyAnimation(&group);
QCOMPARE(group.animationCount(), 2); QCOMPARE(group.animationCount(), 2);