From d5ab0101ffaaa36ccefda80e3a7a1eb5c60070d5 Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Wed, 2 Jun 2021 08:50:35 +0200 Subject: Avoid use after free in tst_qsequentialanimationgroup 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 Reviewed-by: Sona Kurazyan --- src/corelib/animation/qabstractanimation.cpp | 8 ++++++-- .../qsequentialanimationgroup/tst_qsequentialanimationgroup.cpp | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/corelib/animation/qabstractanimation.cpp b/src/corelib/animation/qabstractanimation.cpp index 18dab48e5a..16621036e2 100644 --- a/src/corelib/animation/qabstractanimation.cpp +++ b/src/corelib/animation/qabstractanimation.cpp @@ -1346,6 +1346,12 @@ void QAbstractAnimation::setCurrentTime(int msecs) if (d->currentLoop != oldLoop) 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 // own end state is reached; in this case the animation is time driven, // and has reached the end. @@ -1353,8 +1359,6 @@ void QAbstractAnimation::setCurrentTime(int msecs) || (d->direction == Backward && d->totalCurrentTime == 0)) { stop(); } - if (oldCurrentTime != d->totalCurrentTime) - d->totalCurrentTime.notify(); } /*! diff --git a/tests/auto/corelib/animation/qsequentialanimationgroup/tst_qsequentialanimationgroup.cpp b/tests/auto/corelib/animation/qsequentialanimationgroup/tst_qsequentialanimationgroup.cpp index 43a8593803..779b225bfb 100644 --- a/tests/auto/corelib/animation/qsequentialanimationgroup/tst_qsequentialanimationgroup.cpp +++ b/tests/auto/corelib/animation/qsequentialanimationgroup/tst_qsequentialanimationgroup.cpp @@ -1614,7 +1614,7 @@ void tst_QSequentialAnimationGroup::clear() { SequentialAnimationGroup group; QPointer anim1 = new DummyPropertyAnimation(&group); - group.connect(anim1, SIGNAL(finished()), SLOT(clear())); + connect(anim1, &QAbstractAnimation::finished, &group, &QSequentialAnimationGroup::clear); new DummyPropertyAnimation(&group); QCOMPARE(group.animationCount(), 2); -- cgit v1.2.3