[Webkit-unassigned] [Bug 34566] New: Security: WebCore::FEMorphology::apply memmove ReadAV at NULL (ec3ed2d76f7904e1c4df8ea3b1dd07e6)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34566

           Summary: Security: WebCore::FEMorphology::apply memmove
                    ReadAV at NULL (ec3ed2d76f7904e1c4df8ea3b1dd07e6)
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Windows Vista
            Status: NEW
          Severity: Normal
          Priority: P1
         Component: SVG
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: skylined at chromium.org
                CC: eric at webkit.org


Created an attachment (id=48118)
 --> (https://bugs.webkit.org/attachment.cgi?id=48118)
Repro

Repro.svg:

<?xml version="1.0" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W31.1//EN" "htd">
<svg version="1.1" xmlns:xlink="http://www.w3.org/1999/xlink"
xmlns="http://www.w3.org/2000/svg">
        <filter id="filter">
            <feMorphology operator="erode" radius="-1"/>
        </filter>
        <g id="morphology">
            <path d="M1,2H3v1h1v1z"/>
        </g>
    <use xlink:href="#morphology" filter="url(#filter)"/>
</svg>

The negative radius causes the code to remove element 0 from a vector of 0
elements:

WebKit\WebCore\svg\graphics\filters\SVGFEMorphology.cpp:
void FEMorphology::apply(Filter* filter)
{
<snip>
    if (!m_radiusX || !m_radiusY) // **** possible location for a fix? ****
        return;
<snip>
    Vector<unsigned char> extrema;
<snip>
                if (x - radiusX >= 0)
                    extrema.remove(0); // **** KaB00M ***
<snip>
}

webkit\JavaScriptCore\wtf\Vector.h:

    template<typename T, size_t inlineCapacity>
    inline void Vector<T, inlineCapacity>::remove(size_t position)
    {
        ASSERT(position < size()); // **** THIS SHOULD PROBABLY FAIL HARD IN
RELEASE BUILD ****
        T* spot = begin() + position;
        spot->~T();
        TypeOperations::moveOverlapping(spot + 1, end(), spot); // **** KaB00M:
Reading out of bounds ****
        --m_size;
    }

This consistently causes a NULL pointer read AV in Chromium, so I assume that
an attacker cannot control the memmove and it is not exploitable. However, it
would seem that this is purely by chance and not because we coded it to do so.
Other applications that use WebKit may not be so lucky. I think this ASSERT
should fail hard in any release build, as the condition it checks could lead to
arbitrary code execution.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.