⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 283470 in webkit


Ignore:
Timestamp:
Oct 3, 2021, 10:22:24 AM (5 years ago)
Author:
Simon Fraser
Message:

WebCore::Length incorrectly uses memcpy() for copy constructors/operator and IPC encoding/decoding
​https://bugs.webkit.org/show_bug.cgi?id=230744

Reviewed by David Kilzer.
Source/WebCore:

Copy-constructing Length by memcpy is sketchy; replace with code that initializes the appropriate
fields based on type, taking care to deref() and ref() the calc handle.

Expose isFloat() for encoding.

  • css/parser/CSSPropertyParserHelpers.cpp:

(WebCore::CSSPropertyParserHelpers::CalcParser::consumeValueIfCategory): Add a bit of calc logging.

  • platform/Length.cpp:

(WebCore::Length::Length):

  • platform/Length.h:

(WebCore::Length::Length):
(WebCore::Length::operator=):
(WebCore::Length::isFloat const):

Source/WebKit:

Safe encoding/decoding of Length requires that we encode the enum and fields separately,
and don't allow calc types (there isn't enough context in the receiving process to resolve
calc).

  • Shared/WebCoreArgumentCoders.cpp:

(IPC::ArgumentCoder<Length>::encode):
(IPC::ArgumentCoder<Length>::decode):

  • Shared/WebCoreArgumentCoders.h:
Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r283469 r283470  
     12021-10-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        WebCore::Length incorrectly uses memcpy() for copy constructors/operator and IPC encoding/decoding
     4        https://bugs.webkit.org/show_bug.cgi?id=230744
     5
     6        Reviewed by David Kilzer.
     7
     8        Copy-constructing Length by memcpy is sketchy; replace with code that initializes the appropriate
     9        fields based on type, taking care to deref() and ref() the calc handle.
     10       
     11        Expose isFloat() for encoding.
     12
     13        * css/parser/CSSPropertyParserHelpers.cpp:
     14        (WebCore::CSSPropertyParserHelpers::CalcParser::consumeValueIfCategory): Add a bit of calc logging.
     15        * platform/Length.cpp:
     16        (WebCore::Length::Length):
     17        * platform/Length.h:
     18        (WebCore::Length::Length):
     19        (WebCore::Length::operator=):
     20        (WebCore::Length::isFloat const):
     21
    1222021-10-03  Basuke Suzuki  <basuke.suzuki@sony.com>
    223

  • trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp

    r283279 r283470  
    4646#include "ColorConversion.h"
    4747#include "ColorLuminance.h"
     48#include "Logging.h"
    4849#include "Pair.h"
    4950#include "RenderStyleConstants.h"
    … …  
    5354#include <wtf/SortedArrayMap.h>
    5455#include <wtf/text/StringConcatenateNumbers.h>
     56#include <wtf/text/TextStream.h>
    5557
    5658namespace WebCore {
    … …  
    143145    RefPtr<CSSPrimitiveValue> consumeValueIfCategory(CalculationCategory category)
    144146    {
    145         if (!m_calcValue || m_calcValue->category() != category)
     147        if (!m_calcValue)
    146148            return nullptr;
     149
     150        if (m_calcValue->category() != category) {
     151            LOG_WITH_STREAM(Calc, stream << "CalcParser::consumeValueIfCategory - failing because calc category " << m_calcValue->category() << " does not match requested category " << category);
     152            return nullptr;
     153        }
    147154        m_sourceRange = m_range;
    148155        return m_pool.createValue(WTFMove(m_calcValue));

  • trunk/Source/WebCore/platform/Length.cpp

    r278246 r283470  
    244244
    245245Length::Length(Ref<CalculationValue>&& value)
    246     : m_hasQuirk(false)
    247     , m_type(LengthType::Calculated)
    248     , m_isFloat(false)
     246    : m_type(LengthType::Calculated)
    249247{
    250248    m_calculationValueHandle = calculationValues().insert(WTFMove(value));

  • trunk/Source/WebCore/platform/Length.h

    r278705 r283470  
    2424
    2525#include "AnimationUtilities.h"
    26 #include <memory>
    2726#include <string.h>
    2827#include <wtf/Assertions.h>
    … …  
    3837
    3938enum class LengthType : uint8_t {
    40     Auto, Relative, Percent, Fixed,
    41     Intrinsic, MinIntrinsic,
    42     MinContent, MaxContent, FillAvailable, FitContent,
     39    Auto,
     40    Relative,
     41    Percent,
     42    Fixed,
     43    Intrinsic,
     44    MinIntrinsic,
     45    MinContent,
     46    MaxContent,
     47    FillAvailable,
     48    FitContent,
    4349    Calculated,
    4450    Undefined
    … …  
    7682    void setValue(LengthType, LayoutUnit value);
    7783    Length& operator*=(float);
    78 
    79     void setHasQuirk(bool);
    8084
    8185    bool operator==(const Length&) const;
    … …  
    102106
    103107    bool hasQuirk() const;
     108    void setHasQuirk(bool);
    104109
    105110    // FIXME calc: https://bugs.webkit.org/show_bug.cgi?id=80357. A calculated Length
    … …  
    111116    bool isNegative() const;
    112117
     118    bool isFloat() const;
     119
    113120    bool isPercentOrCalculated() const; // Returns true for both Percent and Calculated.
    114121
    … …  
    125132    bool isCalculatedEqual(const Length&) const;
    126133
     134    void initialize(const Length&);
     135    void initialize(Length&&);
     136
    127137    WEBCORE_EXPORT void ref() const;
    128138    WEBCORE_EXPORT void deref() const;
    129139   
    130140    union {
    131         int m_intValue;
     141        int m_intValue { 0 };
    132142        float m_floatValue;
    133143        unsigned m_calculationValueHandle;
    134144    };
    135     bool m_hasQuirk;
    136145    LengthType m_type;
    137     bool m_isFloat;
     146    bool m_hasQuirk { false };
     147    bool m_isFloat { false };
    138148};
    139149
    … …  
    146156
    147157inline Length::Length(LengthType type)
    148     : m_intValue(0), m_hasQuirk(false), m_type(type), m_isFloat(false)
     158    : m_type(type)
    149159{
    150160    ASSERT(type != LengthType::Calculated);
    … …  
    152162
    153163inline Length::Length(int value, LengthType type, bool hasQuirk)
    154     : m_intValue(value), m_hasQuirk(hasQuirk), m_type(type), m_isFloat(false)
     164    : m_intValue(value)
     165    , m_type(type)
     166    , m_hasQuirk(hasQuirk)
    155167{
    156168    ASSERT(type != LengthType::Calculated);
    … …  
    158170
    159171inline Length::Length(LayoutUnit value, LengthType type, bool hasQuirk)
    160     : m_floatValue(value.toFloat()), m_hasQuirk(hasQuirk), m_type(type), m_isFloat(true)
     172    : m_floatValue(value.toFloat())
     173    , m_type(type)
     174    , m_hasQuirk(hasQuirk)
     175    , m_isFloat(true)
    161176{
    162177    ASSERT(type != LengthType::Calculated);
    … …  
    164179
    165180inline Length::Length(float value, LengthType type, bool hasQuirk)
    166     : m_floatValue(value), m_hasQuirk(hasQuirk), m_type(type), m_isFloat(true)
     181    : m_floatValue(value)
     182    , m_type(type)
     183    , m_hasQuirk(hasQuirk)
     184    , m_isFloat(true)
    167185{
    168186    ASSERT(type != LengthType::Calculated);
    … …  
    170188
    171189inline Length::Length(double value, LengthType type, bool hasQuirk)
    172     : m_floatValue(static_cast<float>(value)), m_hasQuirk(hasQuirk), m_type(type), m_isFloat(true)
     190    : m_floatValue(static_cast<float>(value))
     191    , m_type(type)
     192    , m_hasQuirk(hasQuirk)
     193    , m_isFloat(true)
    173194{
    174195    ASSERT(type != LengthType::Calculated);
    … …  
    177198inline Length::Length(const Length& other)
    178199{
    179     if (other.isCalculated())
    180         other.ref();
    181 
    182     memcpy(static_cast<void*>(this), static_cast<void*>(const_cast<Length*>(&other)), sizeof(Length));
     200    initialize(other);
    183201}
    184202
    185203inline Length::Length(Length&& other)
    186204{
    187     memcpy(static_cast<void*>(this), static_cast<void*>(&other), sizeof(Length));
    188     other.m_type = LengthType::Auto;
     205    initialize(WTFMove(other));
    189206}
    190207
    … …  
    194211        return *this;
    195212
    196     if (other.isCalculated())
    197         other.ref();
    198213    if (isCalculated())
    199214        deref();
    200215
    201     memcpy(static_cast<void*>(this), static_cast<void*>(const_cast<Length*>(&other)), sizeof(Length));
     216    initialize(other);
    202217    return *this;
    203218}
    … …  
    211226        deref();
    212227
    213     memcpy(static_cast<void*>(this), static_cast<void*>(&other), sizeof(Length));
     228    initialize(WTFMove(other));
     229    return *this;
     230}
     231
     232inline void Length::initialize(const Length& other)
     233{
     234    m_type = other.m_type;
     235    m_hasQuirk = other.m_hasQuirk;
     236
     237    switch (m_type) {
     238    case LengthType::Auto:
     239    case LengthType::Undefined:
     240        m_intValue = 0;
     241        break;
     242    case LengthType::Fixed:
     243    case LengthType::Relative:
     244    case LengthType::Intrinsic:
     245    case LengthType::MinIntrinsic:
     246    case LengthType::MinContent:
     247    case LengthType::MaxContent:
     248    case LengthType::FillAvailable:
     249    case LengthType::FitContent:
     250    case LengthType::Percent:
     251        m_isFloat = other.m_isFloat;
     252        if (m_isFloat)
     253            m_floatValue = other.m_floatValue;
     254        else
     255            m_intValue = other.m_intValue;
     256        break;
     257    case LengthType::Calculated:
     258        m_calculationValueHandle = other.m_calculationValueHandle;
     259        ref();
     260        break;
     261    }
     262}
     263
     264inline void Length::initialize(Length&& other)
     265{
     266    m_type = other.m_type;
     267    m_hasQuirk = other.m_hasQuirk;
     268
     269    switch (m_type) {
     270    case LengthType::Auto:
     271    case LengthType::Undefined:
     272        m_intValue = 0;
     273        break;
     274    case LengthType::Fixed:
     275    case LengthType::Relative:
     276    case LengthType::Intrinsic:
     277    case LengthType::MinIntrinsic:
     278    case LengthType::MinContent:
     279    case LengthType::MaxContent:
     280    case LengthType::FillAvailable:
     281    case LengthType::FitContent:
     282    case LengthType::Percent:
     283        m_isFloat = other.m_isFloat;
     284        if (m_isFloat)
     285            m_floatValue = other.m_floatValue;
     286        else
     287            m_intValue = other.m_intValue;
     288        break;
     289    case LengthType::Calculated:
     290        m_calculationValueHandle = std::exchange(other.m_calculationValueHandle, 0);
     291        break;
     292    }
     293
    214294    other.m_type = LengthType::Auto;
    215     return *this;
    216295}
    217296
    … …  
    286365}
    287366
     367inline bool Length::isFloat() const
     368{
     369    return m_isFloat;
     370}
     371
    288372inline void Length::setHasQuirk(bool hasQuirk)
    289373{

  • trunk/Source/WebKit/ChangeLog

    r283457 r283470  
     12021-10-03  Simon Fraser  <simon.fraser@apple.com>
     2
     3        WebCore::Length incorrectly uses memcpy() for copy constructors/operator and IPC encoding/decoding
     4        https://bugs.webkit.org/show_bug.cgi?id=230744
     5
     6        Reviewed by David Kilzer.
     7       
     8        Safe encoding/decoding of Length requires that we encode the enum and fields separately,
     9        and don't allow calc types (there isn't enough context in the receiving process to resolve
     10        calc).
     11
     12        * Shared/WebCoreArgumentCoders.cpp:
     13        (IPC::ArgumentCoder<Length>::encode):
     14        (IPC::ArgumentCoder<Length>::decode):
     15        * Shared/WebCoreArgumentCoders.h:
     16
    1172021-10-02  David Kilzer  <ddkilzer@apple.com>
    218

  • trunk/Source/WebKit/Shared/WebCoreArgumentCoders.cpp

    r283353 r283470  
    843843void ArgumentCoder<Length>::encode(Encoder& encoder, const Length& length)
    844844{
    845     SimpleArgumentCoder<Length>::encode(encoder, length);
     845    encoder << length.type() << length.hasQuirk();
     846
     847    switch (length.type()) {
     848    case LengthType::Auto:
     849    case LengthType::Undefined:
     850        break;
     851    case LengthType::Fixed:
     852    case LengthType::Relative:
     853    case LengthType::Intrinsic:
     854    case LengthType::MinIntrinsic:
     855    case LengthType::MinContent:
     856    case LengthType::MaxContent:
     857    case LengthType::FillAvailable:
     858    case LengthType::FitContent:
     859    case LengthType::Percent:
     860        encoder << length.isFloat();
     861        if (length.isFloat())
     862            encoder << length.value();
     863        else
     864            encoder << length.intValue();
     865        break;
     866    case LengthType::Calculated:
     867        ASSERT_NOT_REACHED();
     868        break;
     869    }
    846870}
    847871
    848872bool ArgumentCoder<Length>::decode(Decoder& decoder, Length& length)
    849873{
    850     return SimpleArgumentCoder<Length>::decode(decoder, length);
     874    LengthType type;
     875    if (!decoder.decode(type))
     876        return false;
     877
     878    bool hasQuirk;
     879    if (!decoder.decode(hasQuirk))
     880        return false;
     881
     882    switch (type) {
     883    case LengthType::Auto:
     884    case LengthType::Undefined:
     885        length = Length(type);
     886        return true;
     887    case LengthType::Fixed:
     888    case LengthType::Relative:
     889    case LengthType::Intrinsic:
     890    case LengthType::MinIntrinsic:
     891    case LengthType::MinContent:
     892    case LengthType::MaxContent:
     893    case LengthType::FillAvailable:
     894    case LengthType::FitContent:
     895    case LengthType::Percent: {
     896        bool isFloat;
     897        if (!decoder.decode(isFloat))
     898            return false;
     899
     900        if (isFloat) {
     901            float value;
     902            if (!decoder.decode(value))
     903                return false;
     904
     905            length = Length(value, type, hasQuirk);
     906        } else {
     907            int value;
     908            if (!decoder.decode(value))
     909                return false;
     910
     911            length = Length(value, type, hasQuirk);
     912        }
     913        return true;
     914    }
     915    case LengthType::Calculated:
     916        ASSERT_NOT_REACHED();
     917        return false;
     918    }
     919
     920    return false;
    851921}
    852922

  • trunk/Source/WebKit/Shared/WebCoreArgumentCoders.h

    r283353 r283470  
    10611061};
    10621062
     1063template<> struct EnumTraits<WebCore::LengthType> {
     1064    using values = EnumValues<
     1065        WebCore::LengthType,
     1066        WebCore::LengthType::Auto,
     1067        WebCore::LengthType::Relative,
     1068        WebCore::LengthType::Percent,
     1069        WebCore::LengthType::Fixed,
     1070        WebCore::LengthType::Intrinsic,
     1071        WebCore::LengthType::MinIntrinsic,
     1072        WebCore::LengthType::MinContent,
     1073        WebCore::LengthType::MaxContent,
     1074        WebCore::LengthType::FillAvailable,
     1075        WebCore::LengthType::FitContent,
     1076        WebCore::LengthType::Calculated,
     1077        WebCore::LengthType::Undefined
     1078    >;
     1079};
     1080
    10631081} // namespace WTF
Note: See TracChangeset for help on using the changeset viewer.