[SCM] WebKit Debian packaging branch, debian/unstable, updated. debian/1.1.15-1-40151-g37bb677

trey trey at 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sat Sep 26 08:44:25 UTC 2009


The following commit has been merged in the debian/unstable branch:
commit b1f6908fba76d8521644318f4a21ef1a53bb2878
Author: trey <trey at 268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Jun 8 19:49:29 2004 +0000

    	Clipboard access during DHTML dragging is made secure.
    
            Reviewed by John.
    
            * kwq/KWQClipboard.h:
            (KWQClipboard::):
            * kwq/KWQClipboard.mm:
            (KWQClipboard::KWQClipboard):    Init new policy and changeCount members.
            (KWQClipboard::becomeNumb):  Set policy to numb.
            (KWQClipboard::clearData):   Check policy.
            (KWQClipboard::clearAllData):   Check policy.
            (KWQClipboard::getData):   Check policy and changeCount.
            (KWQClipboard::setData):   Check policy.
            (KWQClipboard::types):   Check policy and ChangeCount.
            (KWQClipboard::setDragLocation):   Check policy.
            (KWQClipboard::setDragImage):   Check policy.
            (KWQClipboard::setDropEffect):   Check policy.
            (KWQClipboard::setEffectAllowed):   Check policy.
            * kwq/KWQKHTMLPart.mm:
            (KWQKHTMLPart::dispatchDragSrcEvent):  Set policy when creating clipboard,
    	and numb-ify it when we're done.
            * kwq/WebCoreBridge.mm:
            (-[WebCoreBridge dragOperationForDraggingInfo:]): Set policy when creating clipboard,
    	and numb-ify it when we're done.
            (-[WebCoreBridge dragExitedWithDraggingInfo:]):  Ditto
            (-[WebCoreBridge concludeDragForDraggingInfo:]):  Ditto
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@6791 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index 9403454..f9e6b49 100644
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,32 @@
+2004-06-07  Trey Matteson  <trey at apple.com>
+
+	Clipboard access during DHTML dragging is made secure.
+
+        Reviewed by John.
+
+        * kwq/KWQClipboard.h:
+        (KWQClipboard::):
+        * kwq/KWQClipboard.mm:
+        (KWQClipboard::KWQClipboard):    Init new policy and changeCount members.
+        (KWQClipboard::becomeNumb):  Set policy to numb.
+        (KWQClipboard::clearData):   Check policy.
+        (KWQClipboard::clearAllData):   Check policy.
+        (KWQClipboard::getData):   Check policy and changeCount.
+        (KWQClipboard::setData):   Check policy.
+        (KWQClipboard::types):   Check policy and ChangeCount.
+        (KWQClipboard::setDragLocation):   Check policy.
+        (KWQClipboard::setDragImage):   Check policy.
+        (KWQClipboard::setDropEffect):   Check policy.
+        (KWQClipboard::setEffectAllowed):   Check policy.
+        * kwq/KWQKHTMLPart.mm:
+        (KWQKHTMLPart::dispatchDragSrcEvent):  Set policy when creating clipboard,
+	and numb-ify it when we're done.
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge dragOperationForDraggingInfo:]): Set policy when creating clipboard,
+	and numb-ify it when we're done.
+        (-[WebCoreBridge dragExitedWithDraggingInfo:]):  Ditto
+        (-[WebCoreBridge concludeDragForDraggingInfo:]):  Ditto
+
 2004-06-08  Ken Kocienda  <kocienda at apple.com>
 
         Reviewed by me
diff --git a/WebCore/kwq/KWQClipboard.h b/WebCore/kwq/KWQClipboard.h
index d52f8a5..0143f1a 100644
--- a/WebCore/kwq/KWQClipboard.h
+++ b/WebCore/kwq/KWQClipboard.h
@@ -34,7 +34,12 @@
 class KWQClipboard : public DOM::ClipboardImpl
 {
 public:
-    KWQClipboard(bool forDragging, NSPasteboard *pasteboard);
+    // security mechanism
+    typedef enum {
+        Numb, Writable, TypesReadable, Readable
+    } AccessPolicy;
+
+    KWQClipboard(bool forDragging, NSPasteboard *pasteboard, AccessPolicy policy);
     virtual ~KWQClipboard();
 
     bool isForDragging() const;
@@ -48,9 +53,7 @@ public:
     void clearAllData();
     DOM::DOMString getData(const DOM::DOMString &type, bool &success) const;
     bool setData(const DOM::DOMString &type, const DOM::DOMString &data);
-    
-    //FIXME: need invalidate method for security
-    
+        
     // extensions beyond IE's API
     virtual QStringList types() const;
 
@@ -66,6 +69,9 @@ public:
     void setSourceOperation(NSDragOperation op);
     void setDestinationOperation(NSDragOperation op);
 
+    // sets AccessPolicy = Numb - trap door, once this is set, no going back
+    void becomeNumb();
+
 private:
     NSPasteboard *m_pasteboard;
     bool m_forDragging;
@@ -73,6 +79,8 @@ private:
     DOM::DOMString m_effectAllowed;
     QPoint m_dragLoc;
     QPixmap m_dragImage;
+    AccessPolicy m_policy;
+    int m_changeCount;
 };
 
 
diff --git a/WebCore/kwq/KWQClipboard.mm b/WebCore/kwq/KWQClipboard.mm
index 10ebc86..4b643c9 100644
--- a/WebCore/kwq/KWQClipboard.mm
+++ b/WebCore/kwq/KWQClipboard.mm
@@ -28,9 +28,10 @@
 
 using DOM::DOMString;
 
-KWQClipboard::KWQClipboard(bool forDragging, NSPasteboard *pasteboard)
-  : m_pasteboard([pasteboard retain]), m_forDragging(forDragging)
+KWQClipboard::KWQClipboard(bool forDragging, NSPasteboard *pasteboard, AccessPolicy policy)
+  : m_pasteboard([pasteboard retain]), m_forDragging(forDragging), m_policy(policy)
 {
+    m_changeCount = [m_pasteboard changeCount];
 }
 
 KWQClipboard::~KWQClipboard()
@@ -43,6 +44,11 @@ bool KWQClipboard::isForDragging() const
     return m_forDragging;
 }
 
+void KWQClipboard::becomeNumb()
+{
+    m_policy = Numb;
+}
+
 // FIXME hardwired for now, will use UTI
 static NSString *cocoaTypeFromMIMEType(const DOMString &type) {
     QString qType = type.string();
@@ -95,6 +101,11 @@ static QString MIMETypeFromCocoaType(NSString *type)
 
 void KWQClipboard::clearData(const DOMString &type)
 {
+    if (m_policy != Writable) {
+        return;
+    }
+    // note NSPasteboard enforces changeCount itself on writing - can't write if not the owner
+
     NSString *cocoaType = cocoaTypeFromMIMEType(type);
     if (cocoaType) {
         [m_pasteboard setString:@"" forType:cocoaType];
@@ -103,12 +114,21 @@ void KWQClipboard::clearData(const DOMString &type)
 
 void KWQClipboard::clearAllData()
 {
+    if (m_policy != Writable) {
+        return;
+    }
+    // note NSPasteboard enforces changeCount itself on writing - can't write if not the owner
+
     [m_pasteboard declareTypes:[NSArray array] owner:nil];
 }
 
 DOMString KWQClipboard::getData(const DOMString &type, bool &success) const
 {
     success = false;
+    if (m_policy != Readable) {
+        return DOMString();
+    }
+    
     NSString *cocoaType = cocoaTypeFromMIMEType(type);
     NSString *cocoaValue = nil;
     NSArray *availableTypes = [m_pasteboard types];
@@ -153,7 +173,9 @@ DOMString KWQClipboard::getData(const DOMString &type, bool &success) const
         cocoaValue = [m_pasteboard stringForType:cocoaType];
     }
 
-    if (cocoaValue) {
+    // Enforce changeCount ourselves for security.  We check after reading instead of before to be
+    // sure it doesn't change between our testing the change count and accessing the data.
+    if (cocoaValue && m_changeCount == [m_pasteboard changeCount]) {
         success = true;
         return DOMString(QString::fromNSString(cocoaValue));
     } else {
@@ -163,6 +185,11 @@ DOMString KWQClipboard::getData(const DOMString &type, bool &success) const
 
 bool KWQClipboard::setData(const DOMString &type, const DOMString &data)
 {
+    if (m_policy != Writable) {
+        return false;
+    }
+    // note NSPasteboard enforces changeCount itself on writing - can't write if not the owner
+
     NSString *cocoaType = cocoaTypeFromMIMEType(type);
     NSString *cocoaData = data.string().getNSString();
     if (cocoaType == NSURLPboardType) {
@@ -189,7 +216,18 @@ bool KWQClipboard::setData(const DOMString &type, const DOMString &data)
 
 QStringList KWQClipboard::types() const
 {
+    if (m_policy != Readable && m_policy != TypesReadable) {
+        return QStringList();
+    }
+
     NSArray *types = [m_pasteboard types];
+
+    // Enforce changeCount ourselves for security.  We check after reading instead of before to be
+    // sure it doesn't change between our testing the change count and accessing the data.
+    if (m_changeCount != [m_pasteboard changeCount]) {
+        return QStringList();
+    }
+
     QStringList result;
     if (types) {
         unsigned count = [types count];
@@ -204,6 +242,8 @@ QStringList KWQClipboard::types() const
     return result;
 }
 
+// The rest of these getters don't really have any impact on security, so for now make no checks
+
 QPoint KWQClipboard::dragLocation() const
 {
     return m_dragLoc;
@@ -211,7 +251,9 @@ QPoint KWQClipboard::dragLocation() const
 
 void KWQClipboard::setDragLocation(const QPoint &p)
 {
-    m_dragLoc = p;
+    if (m_policy == Writable) {
+        m_dragLoc = p;
+    }
 }
 
 QPixmap KWQClipboard::dragImage() const
@@ -221,7 +263,9 @@ QPixmap KWQClipboard::dragImage() const
 
 void KWQClipboard::setDragImage(const QPixmap &pm)
 {
-    m_dragImage = pm;
+    if (m_policy == Writable) {
+        m_dragImage = pm;
+    }
 }
 
 NSImage *KWQClipboard::dragNSImage()
@@ -236,7 +280,9 @@ DOM::DOMString KWQClipboard::dropEffect() const
 
 void KWQClipboard::setDropEffect(const DOM::DOMString &s)
 {
-    m_dropEffect = s;
+    if (m_policy == Writable) {
+        m_dropEffect = s;
+    }
 }
 
 DOM::DOMString KWQClipboard::effectAllowed() const
@@ -246,9 +292,14 @@ DOM::DOMString KWQClipboard::effectAllowed() const
 
 void KWQClipboard::setEffectAllowed(const DOM::DOMString &s)
 {
-    m_effectAllowed = s;
+    if (m_policy == Writable) {
+        m_effectAllowed = s;
+    }
 }
 
+// These "conversion" methods are called by the bridge and part, and never make sense to JS, so we don't
+// worry about security for these.  The don't allow access to the pasteboard anyway.
+
 static NSDragOperation cocoaOpFromIEOp(const DOMString &op) {
     // yep, it's really just this fixed set
     if (op == "none") {
diff --git a/WebCore/kwq/KWQKHTMLPart.mm b/WebCore/kwq/KWQKHTMLPart.mm
index 0129070..79c9ebe 100644
--- a/WebCore/kwq/KWQKHTMLPart.mm
+++ b/WebCore/kwq/KWQKHTMLPart.mm
@@ -1914,10 +1914,10 @@ bool KWQKHTMLPart::dispatchDragSrcEvent(int eventId, const QPoint &loc, bool dec
         // also done for security, as it erases data from the last drag
         [pasteboard declareTypes:[NSArray array] owner:nil];
     }
-    KWQClipboard *clipboard = new KWQClipboard(true, pasteboard);
+    KWQClipboard *clipboard = new KWQClipboard(true, pasteboard, KWQClipboard::Writable);
     clipboard->ref();
     bool DHTMLBailed = d->m_view->dispatchDragEvent(eventId, _dragSrc.handle(), loc, clipboard);
-    // FIXME - invalidate clipboard here for security
+    clipboard->becomeNumb();    // invalidate clipboard here for security
 
     if (dragImage) {
         *dragImage = clipboard->dragNSImage();
diff --git a/WebCore/kwq/WebCoreBridge.mm b/WebCore/kwq/WebCoreBridge.mm
index 465f638..4c62ddb 100644
--- a/WebCore/kwq/WebCoreBridge.mm
+++ b/WebCore/kwq/WebCoreBridge.mm
@@ -1560,7 +1560,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
             // Sending an event can result in the destruction of the view and part.
             v->ref();
             
-            KWQClipboard *clipboard = new KWQClipboard(true, [info draggingPasteboard]);
+            KWQClipboard *clipboard = new KWQClipboard(true, [info draggingPasteboard], KWQClipboard::TypesReadable);
             clipboard->ref();
             NSDragOperation srcOp = [info draggingSourceOperationMask];
             clipboard->setSourceOperation(srcOp);
@@ -1585,6 +1585,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
                     op = NSDragOperationNone;
                 }
             }
+            clipboard->becomeNumb();    // invalidate clipboard here for security
 
             clipboard->deref();
             v->deref();
@@ -1602,10 +1603,11 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
             // Sending an event can result in the destruction of the view and part.
             v->ref();
 
-            KWQClipboard *clipboard = new KWQClipboard(true, [info draggingPasteboard]);
+            KWQClipboard *clipboard = new KWQClipboard(true, [info draggingPasteboard], KWQClipboard::TypesReadable);
             clipboard->ref();
             
             v->cancelDragAndDrop(QPoint([info draggingLocation]), clipboard);
+            clipboard->becomeNumb();    // invalidate clipboard here for security
 
             clipboard->deref();
             v->deref();
@@ -1621,10 +1623,11 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
             // Sending an event can result in the destruction of the view and part.
             v->ref();
 
-            KWQClipboard *clipboard = new KWQClipboard(true, [info draggingPasteboard]);
+            KWQClipboard *clipboard = new KWQClipboard(true, [info draggingPasteboard], KWQClipboard::Readable);
             clipboard->ref();
 
             BOOL result = v->performDragAndDrop(QPoint([info draggingLocation]), clipboard);
+            clipboard->becomeNumb();    // invalidate clipboard here for security
 
             clipboard->deref();
             v->deref();

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list