Skip to content

Commit

Permalink
Cherry-pick 259548.723@safari-7615-branch (9e8e582). https://bugs.web…
Browse files Browse the repository at this point in the history 
…kit.org/show_bug.cgi?id=255629

    TransformOperation subclasses should verify deserialized type
    https://bugs.webkit.org/show_bug.cgi?id=255629
    rdar://108161092

    Reviewed by David Kilzer.

    The type needs to line up with the same types used by the is, downcast, and dynamicDowncast functions.

    * Source/WebCore/animation/KeyframeEffect.cpp:
    (WebCore::KeyframeEffect::computedNeedsForcedLayout):
    * Source/WebCore/platform/graphics/transforms/IdentityTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/Matrix3DTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/MatrixTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/PerspectiveTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/RotateTransformOperation.cpp:
    (WebCore::RotateTransformOperation::RotateTransformOperation):
    * Source/WebCore/platform/graphics/transforms/RotateTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/ScaleTransformOperation.cpp:
    (WebCore::ScaleTransformOperation::ScaleTransformOperation):
    * Source/WebCore/platform/graphics/transforms/ScaleTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/SkewTransformOperation.cpp:
    (WebCore::SkewTransformOperation::SkewTransformOperation):
    * Source/WebCore/platform/graphics/transforms/SkewTransformOperation.h:
    * Source/WebCore/platform/graphics/transforms/TransformOperation.h:
    (WebCore::TransformOperation::isRotateTransformOperationType):
    (WebCore::TransformOperation::isScaleTransformOperationType):
    (WebCore::TransformOperation::isSkewTransformOperationType):
    (WebCore::TransformOperation::isTranslateTransformOperationType):
    (WebCore::TransformOperation::isRotateTransformOperationType const): Deleted.
    (WebCore::TransformOperation::isScaleTransformOperationType const): Deleted.
    (WebCore::TransformOperation::isSkewTransformOperationType const): Deleted.
    (WebCore::TransformOperation::isTranslateTransformOperationType const): Deleted.
    * Source/WebCore/platform/graphics/transforms/TranslateTransformOperation.cpp:
    (WebCore::TranslateTransformOperation::TranslateTransformOperation):
    * Source/WebCore/platform/graphics/transforms/TranslateTransformOperation.h:
    * Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:

    Canonical link: https://commits.webkit.org/259548.723@safari-7615-branch
  • Loading branch information
@achristensen07 @mcatanzaro
achristensen07 authored and mcatanzaro committed Jul 28, 2023
1 parent 0fe7013 commit e5958f9
Show file tree
Hide file tree
Showing 15 changed files with 40 additions and 27 deletions.
3 changes: 1 addition & 2 deletions Source/WebCore/animation/KeyframeEffect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1097,8 +1097,7 @@ void KeyframeEffect::computedNeedsForcedLayout()
if (keyframeStyle->hasTransform()) {
auto& transformOperations = keyframeStyle->transform();
for (const auto& operation : transformOperations.operations()) {
if (operation->isTranslateTransformOperationType()) {
auto translation = downcast<TranslateTransformOperation>(operation.get());
if (auto* translation = dynamicDowncast<TranslateTransformOperation>(operation.get())) {
if (translation->x().isPercent() || translation->y().isPercent()) {
m_needsForcedLayout = true;
return;
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/IdentityTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,4 @@ class IdentityTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::IdentityTransformOperation, type() == WebCore::TransformOperation::Type::Identity)
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::IdentityTransformOperation, WebCore::TransformOperation::Type::Identity ==)
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/Matrix3DTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,4 @@ class Matrix3DTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::Matrix3DTransformOperation, type() == WebCore::TransformOperation::Type::Matrix3D)
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::Matrix3DTransformOperation, WebCore::TransformOperation::Type::Matrix3D ==)
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/MatrixTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,4 @@ class MatrixTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::MatrixTransformOperation, type() == WebCore::TransformOperation::Type::Matrix)
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::MatrixTransformOperation, WebCore::TransformOperation::Type::Matrix ==)
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/PerspectiveTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,4 @@ class PerspectiveTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::PerspectiveTransformOperation, type() == WebCore::TransformOperation::Type::Perspective)
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::PerspectiveTransformOperation, WebCore::TransformOperation::Type::Perspective ==)
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/RotateTransformOperation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ RotateTransformOperation::RotateTransformOperation(double x, double y, double z,
, m_z(z)
, m_angle(angle)
{
ASSERT(isRotateTransformOperationType());
RELEASE_ASSERT(isRotateTransformOperationType(type));
}

bool RotateTransformOperation::operator==(const TransformOperation& other) const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/RotateTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,4 @@ class RotateTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::RotateTransformOperation, isRotateTransformOperationType())
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::RotateTransformOperation, WebCore::TransformOperation::isRotateTransformOperationType)
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/ScaleTransformOperation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ ScaleTransformOperation::ScaleTransformOperation(double sx, double sy, double sz
, m_y(sy)
, m_z(sz)
{
ASSERT(isScaleTransformOperationType());
RELEASE_ASSERT(isScaleTransformOperationType(type));
}

bool ScaleTransformOperation::operator==(const TransformOperation& other) const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/ScaleTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,4 @@ class ScaleTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::ScaleTransformOperation, isScaleTransformOperationType())
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::ScaleTransformOperation, WebCore::TransformOperation::isScaleTransformOperationType)
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/SkewTransformOperation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ SkewTransformOperation::SkewTransformOperation(double angleX, double angleY, Tra
, m_angleX(angleX)
, m_angleY(angleY)
{
ASSERT(isSkewTransformOperationType());
RELEASE_ASSERT(isSkewTransformOperationType(type));
}

bool SkewTransformOperation::operator==(const TransformOperation& other) const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/SkewTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,4 @@ class SkewTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::SkewTransformOperation, isSkewTransformOperationType())
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::SkewTransformOperation, WebCore::TransformOperation::isSkewTransformOperationType)
32 changes: 23 additions & 9 deletions Source/WebCore/platform/graphics/transforms/TransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,24 +107,38 @@ class TransformOperation : public RefCounted<TransformOperation> {

virtual bool isRepresentableIn2D() const { return true; }

bool isRotateTransformOperationType() const
static bool isRotateTransformOperationType(Type type)
{
return type() == Type::RotateX || type() == Type::RotateY || type() == Type::RotateZ || type() == Type::Rotate || type() == Type::Rotate3D;
return type == Type::RotateX
|| type == Type::RotateY
|| type == Type::RotateZ
|| type == Type::Rotate
|| type == Type::Rotate3D;
}

bool isScaleTransformOperationType() const
static bool isScaleTransformOperationType(Type type)
{
return type() == Type::ScaleX || type() == Type::ScaleY || type() == Type::ScaleZ || type() == Type::Scale || type() == Type::Scale3D;
return type == Type::ScaleX
|| type == Type::ScaleY
|| type == Type::ScaleZ
|| type == Type::Scale
|| type == Type::Scale3D;
}

bool isSkewTransformOperationType() const
static bool isSkewTransformOperationType(Type type)
{
return type() == Type::SkewX || type() == Type::SkewY || type() == Type::Skew;
return type == Type::SkewX
|| type == Type::SkewY
|| type == Type::Skew;
}

bool isTranslateTransformOperationType() const
static bool isTranslateTransformOperationType(Type type)
{
return type() == Type::TranslateX || type() == Type::TranslateY || type() == Type::TranslateZ || type() == Type::Translate || type() == Type::Translate3D;
return type == Type::TranslateX
|| type == Type::TranslateY
|| type == Type::TranslateZ
|| type == Type::Translate
|| type == Type::Translate3D;
}

virtual void dump(WTF::TextStream&) const = 0;
Expand Down Expand Up @@ -173,5 +187,5 @@ template<> struct EnumTraits<WebCore::TransformOperation::Type> {

#define SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(ToValueTypeName, predicate) \
SPECIALIZE_TYPE_TRAITS_BEGIN(ToValueTypeName) \
static bool isType(const WebCore::TransformOperation& operation) { return operation.predicate; } \
static bool isType(const WebCore::TransformOperation& operation) { return predicate(operation.type()); } \
SPECIALIZE_TYPE_TRAITS_END()
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/TranslateTransformOperation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ TranslateTransformOperation::TranslateTransformOperation(const Length& tx, const
, m_y(ty)
, m_z(tz)
{
ASSERT(isTranslateTransformOperationType());
RELEASE_ASSERT(isTranslateTransformOperationType(type));
}

bool TranslateTransformOperation::operator==(const TransformOperation& other) const
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/graphics/transforms/TranslateTransformOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,4 @@ class TranslateTransformOperation final : public TransformOperation {

} // namespace WebCore

SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::TranslateTransformOperation, isTranslateTransformOperationType())
SPECIALIZE_TYPE_TRAITS_TRANSFORMOPERATION(WebCore::TranslateTransformOperation, WebCore::TransformOperation::isTranslateTransformOperationType)
8 changes: 4 additions & 4 deletions Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1889,28 +1889,28 @@ header: <WebCore/KeyboardScroll.h>
WebCore::Length x();
WebCore::Length y();
WebCore::Length z();
WebCore::TransformOperation::Type type();
[Validator='WebCore::TransformOperation::isTranslateTransformOperationType(*type)'] WebCore::TransformOperation::Type type();
}

[RefCounted] class WebCore::RotateTransformOperation {
double x();
double y();
double z();
double angle();
WebCore::TransformOperation::Type type();
[Validator='WebCore::TransformOperation::isRotateTransformOperationType(*type)'] WebCore::TransformOperation::Type type();
}

[RefCounted] class WebCore::ScaleTransformOperation {
double x();
double y();
double z();
WebCore::TransformOperation::Type type();
[Validator='WebCore::TransformOperation::isScaleTransformOperationType(*type)'] WebCore::TransformOperation::Type type();
}

[RefCounted] class WebCore::SkewTransformOperation {
double angleX();
double angleY();
WebCore::TransformOperation::Type type();
[Validator='WebCore::TransformOperation::isSkewTransformOperationType(*type)'] WebCore::TransformOperation::Type type();
}

[RefCounted] class WebCore::PerspectiveTransformOperation {
Expand Down

2 comments on commit e5958f9

@uninsane
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcatanzaro are you able to fix the permissions for that bug report linked in the commit message? several of my applications embed webkitgtk and i'd like to gauge how likely i am to have been impacted by the RCEs announced alongside this latest update -- and how i should adjust my practices to decrease the impact of future similar bugs -- but that takes a lot more work for me and i'm more likely to miss something important when the bug reports are marked private.

@mcatanzaro
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @uninsane, WebKit generally does not make security bug reports public. Sorry. I think it makes sense to make them public after the fix has shipped, but it's not really up to me.

What I can tell you is there is no useful information of any sort in the bug report, so it wouldn't make any difference in this case. The only comments in the bug report are links to internal Apple issue trackers and internal Apple GitHub, so whatever discussion exists must be private to Apple only. I don't have any information about this vulnerability other than what you see in the commit message here.

Remote code execution vulnerabilities are very common in WebKit, as in all web engines. It won't be very long before the next batch of issues drops. Regularly updating to the latest stable version of WebKitGTK is highly recommended. I also strongly recommend enabling the web process sandbox (webkit_web_context_set_sandbox_enabled()) if you're using the older webkit2gtk-4.0 or webkit2gtk-4.1 API versions (it is always enabled if you're using the latest webkitgtk-6.0 API version).

Please sign in to comment.