Bug 631946 - SVG DOM lists should limit the number of items to the number they can index. r=roc, a=roc.
authorJonathan Watt <jwatt@jwatt.org>
Tue, 08 Feb 2011 13:43:34 +0000
changeset 62138 5268e56c2b267c0e04712acfa741a046d4461fc1
parent 62137 7f6b89af70745490e1baeddac84849290c062c27
child 62139 5cdf898ba382f6c31f677445619da33da20e7465
push idunknown
push userunknown
push dateunknown
reviewersroc, roc
bugs631946
milestone2.0b12pre
Bug 631946 - SVG DOM lists should limit the number of items to the number they can index. r=roc, a=roc.
content/svg/content/src/DOMSVGLength.cpp
content/svg/content/src/DOMSVGLength.h
content/svg/content/src/DOMSVGLengthList.cpp
content/svg/content/src/DOMSVGNumber.cpp
content/svg/content/src/DOMSVGNumber.h
content/svg/content/src/DOMSVGNumberList.cpp
content/svg/content/src/DOMSVGPathSeg.cpp
content/svg/content/src/DOMSVGPathSeg.h
content/svg/content/src/DOMSVGPathSegList.cpp
content/svg/content/src/DOMSVGPoint.h
content/svg/content/src/DOMSVGPointList.cpp
--- a/content/svg/content/src/DOMSVGLength.cpp
+++ b/content/svg/content/src/DOMSVGLength.cpp
@@ -86,17 +86,17 @@ DOMSVGLength::DOMSVGLength(DOMSVGLengthL
   , mAttrEnum(aAttrEnum)
   , mIsAnimValItem(aIsAnimValItem)
   , mUnit(nsIDOMSVGLength::SVG_LENGTHTYPE_NUMBER)
   , mValue(0.0f)
 {
   // These shifts are in sync with the members in the header.
   NS_ABORT_IF_FALSE(aList &&
                     aAttrEnum < (1 << 4) &&
-                    aListIndex < (1 << 22) &&
+                    aListIndex <= MaxListIndex() &&
                     aIsAnimValItem < (1 << 1), "bad arg");
 
   NS_ABORT_IF_FALSE(IndexIsValid(), "Bad index for DOMSVGNumber!");
 }
 
 DOMSVGLength::DOMSVGLength()
   : mList(nsnull)
   , mListIndex(0)
--- a/content/svg/content/src/DOMSVGLength.h
+++ b/content/svg/content/src/DOMSVGLength.h
@@ -48,16 +48,18 @@ class nsSVGElement;
 // We make DOMSVGLength a pseudo-interface to allow us to QI to it in order to
 // check that the objects that scripts pass to DOMSVGLengthList methods are our
 // *native* length objects.
 //
 // {A8468350-7F7B-4976-9A7E-3765A1DADF9A}
 #define MOZILLA_DOMSVGLENGTH_IID \
   { 0xA8468350, 0x7F7B, 0x4976, { 0x9A, 0x7E, 0x37, 0x65, 0xA1, 0xDA, 0xDF, 0x9A } }
 
+#define MOZ_SVG_LIST_INDEX_BIT_COUNT 22 // supports > 4 million list items
+
 namespace mozilla {
 
 /**
  * Class DOMSVGLength
  *
  * This class creates the DOM objects that wrap internal SVGLength objects that
  * are in an SVGLengthList. It is also used to create the objects returned by
  * SVGSVGElement.createSVGLength().
@@ -156,16 +158,20 @@ public:
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGLengthList).)
    */
   void InsertingIntoList(DOMSVGLengthList *aList,
                          PRUint8 aAttrEnum,
                          PRUint32 aListIndex,
                          PRUint8 aIsAnimValItem);
 
+  static PRUint32 MaxListIndex() {
+    return (1U << MOZ_SVG_LIST_INDEX_BIT_COUNT) - 1;
+  }
+
   /// This method is called to notify this object that its list index changed.
   void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
@@ -209,22 +215,24 @@ private:
   PRBool IndexIsValid();
 #endif
 
   nsRefPtr<DOMSVGLengthList> mList;
 
   // Bounds for the following are checked in the ctor, so be sure to update
   // that if you change the capacity of any of the following.
 
-  PRUint32 mListIndex:22; // supports > 4 million list items
+  PRUint32 mListIndex:MOZ_SVG_LIST_INDEX_BIT_COUNT;
   PRUint32 mAttrEnum:4; // supports up to 16 attributes
   PRUint32 mIsAnimValItem:1;
 
   // The following members are only used when we're not in a list:
   PRUint32 mUnit:5; // can handle 31 units (the 10 SVG 1.1 units + rem, vw, vh, wm, calc + future additions)
   float mValue;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(DOMSVGLength, MOZILLA_DOMSVGLENGTH_IID)
 
 } // namespace mozilla
 
+#undef MOZ_SVG_LIST_INDEX_BIT_COUNT
+
 #endif // MOZILLA_DOMSVGLENGTH_H__
--- a/content/svg/content/src/DOMSVGLengthList.cpp
+++ b/content/svg/content/src/DOMSVGLengthList.cpp
@@ -73,16 +73,22 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END
 
 
 void
 DOMSVGLengthList::InternalListLengthWillChange(PRUint32 aNewLength)
 {
   PRUint32 oldLength = mItems.Length();
 
+  if (aNewLength > DOMSVGLength::MaxListIndex()) {
+    // It's safe to get out of sync with our internal list as long as we have
+    // FEWER items than it does.
+    aNewLength = DOMSVGLength::MaxListIndex();
+  }
+
   // If our length will decrease, notify the items that will be removed:
   for (PRUint32 i = aNewLength; i < oldLength; ++i) {
     if (mItems[i]) {
       mItems[i]->RemovingFromList();
     }
   }
 
   if (!mItems.SetLength(aNewLength)) {
@@ -197,24 +203,28 @@ DOMSVGLengthList::InsertItemBefore(nsIDO
                                    PRUint32 index,
                                    nsIDOMSVGLength **_retval)
 {
   *_retval = nsnull;
   if (IsAnimValList()) {
     return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
   }
 
+  index = NS_MIN(index, Length());
+  if (index >= DOMSVGLength::MaxListIndex()) {
+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
+  }
+
   nsCOMPtr<DOMSVGLength> domItem = do_QueryInterface(newItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
   if (domItem->HasOwner()) {
     domItem = domItem->Copy(); // must do this before changing anything!
   }
-  index = NS_MIN(index, Length());
 
   // Ensure we have enough memory so we can avoid complex error handling below:
   if (!mItems.SetCapacity(mItems.Length() + 1) ||
       !InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   InternalList().InsertItem(index, domItem->ToSVGLength());
--- a/content/svg/content/src/DOMSVGNumber.cpp
+++ b/content/svg/content/src/DOMSVGNumber.cpp
@@ -82,17 +82,17 @@ DOMSVGNumber::DOMSVGNumber(DOMSVGNumberL
   , mListIndex(aListIndex)
   , mAttrEnum(aAttrEnum)
   , mIsAnimValItem(aIsAnimValItem)
   , mValue(0.0f)
 {
   // These shifts are in sync with the members in the header.
   NS_ABORT_IF_FALSE(aList &&
                     aAttrEnum < (1 << 4) &&
-                    aListIndex < (1 << 27) &&
+                    aListIndex <= MaxListIndex() &&
                     aIsAnimValItem < (1 << 1), "bad arg");
 
   NS_ABORT_IF_FALSE(IndexIsValid(), "Bad index for DOMSVGNumber!");
 }
 
 DOMSVGNumber::DOMSVGNumber()
   : mList(nsnull)
   , mListIndex(0)
--- a/content/svg/content/src/DOMSVGNumber.h
+++ b/content/svg/content/src/DOMSVGNumber.h
@@ -48,16 +48,18 @@ class nsSVGElement;
 // check that the objects that scripts pass to DOMSVGNumberList methods are our
 // *native* number objects.
 //
 // {2CA92412-2E1F-4DDB-A16C-52B3B582270D}
 #define MOZILLA_DOMSVGNUMBER_IID \
   { 0x2CA92412, 0x2E1F, 0x4DDB, \
     { 0xA1, 0x6C, 0x52, 0xB3, 0xB5, 0x82, 0x27, 0x0D } }
 
+#define MOZ_SVG_LIST_INDEX_BIT_COUNT 27 // supports > 134 million list items
+
 namespace mozilla {
 
 /**
  * Class DOMSVGNumber
  *
  * This class creates the DOM objects that wrap internal SVGNumber objects that
  * are in an SVGNumberList. It is also used to create the objects returned by
  * SVGSVGElement.createSVGNumber().
@@ -129,16 +131,20 @@ public:
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGNumberList).)
    */
   void InsertingIntoList(DOMSVGNumberList *aList,
                          PRUint8 aAttrEnum,
                          PRUint32 aListIndex,
                          PRUint8 aIsAnimValItem);
 
+  static PRUint32 MaxListIndex() {
+    return (1U << MOZ_SVG_LIST_INDEX_BIT_COUNT) - 1;
+  }
+
   /// This method is called to notify this object that its list index changed.
   void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
@@ -174,21 +180,23 @@ private:
   PRBool IndexIsValid();
 #endif
 
   nsRefPtr<DOMSVGNumberList> mList;
 
   // Bounds for the following are checked in the ctor, so be sure to update
   // that if you change the capacity of any of the following.
 
-  PRUint32 mListIndex:27; // supports > 134 million list items
+  PRUint32 mListIndex:MOZ_SVG_LIST_INDEX_BIT_COUNT;
   PRUint32 mAttrEnum:4; // supports up to 16 attributes
   PRUint32 mIsAnimValItem:1;
 
   // The following member is only used when we're not in a list:
   float mValue;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(DOMSVGNumber, MOZILLA_DOMSVGNUMBER_IID)
 
 } // namespace mozilla
 
+#undef MOZ_SVG_LIST_INDEX_BIT_COUNT
+
 #endif // MOZILLA_DOMSVGNUMBER_H__
--- a/content/svg/content/src/DOMSVGNumberList.cpp
+++ b/content/svg/content/src/DOMSVGNumberList.cpp
@@ -71,16 +71,22 @@ NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(
 NS_INTERFACE_MAP_END
 
 
 void
 DOMSVGNumberList::InternalListLengthWillChange(PRUint32 aNewLength)
 {
   PRUint32 oldLength = mItems.Length();
 
+  if (aNewLength > DOMSVGNumber::MaxListIndex()) {
+    // It's safe to get out of sync with our internal list as long as we have
+    // FEWER items than it does.
+    aNewLength = DOMSVGNumber::MaxListIndex();
+  }
+
   // If our length will decrease, notify the items that will be removed:
   for (PRUint32 i = aNewLength; i < oldLength; ++i) {
     if (mItems[i]) {
       mItems[i]->RemovingFromList();
     }
   }
 
   if (!mItems.SetLength(aNewLength)) {
@@ -195,24 +201,28 @@ DOMSVGNumberList::InsertItemBefore(nsIDO
                                    PRUint32 index,
                                    nsIDOMSVGNumber **_retval)
 {
   *_retval = nsnull;
   if (IsAnimValList()) {
     return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
   }
 
+  index = NS_MIN(index, Length());
+  if (index >= DOMSVGNumber::MaxListIndex()) {
+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
+  }
+
   nsCOMPtr<DOMSVGNumber> domItem = do_QueryInterface(newItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
   if (domItem->HasOwner()) {
     domItem = domItem->Clone(); // must do this before changing anything!
   }
-  index = NS_MIN(index, Length());
 
   // Ensure we have enough memory so we can avoid complex error handling below:
   if (!mItems.SetCapacity(mItems.Length() + 1) ||
       !InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   InternalList().InsertItem(index, domItem->ToSVGNumber());
--- a/content/svg/content/src/DOMSVGPathSeg.cpp
+++ b/content/svg/content/src/DOMSVGPathSeg.cpp
@@ -78,17 +78,17 @@ DOMSVGPathSeg::DOMSVGPathSeg(DOMSVGPathS
                              PRUint32 aListIndex,
                              PRBool aIsAnimValItem)
   : mList(aList)
   , mListIndex(aListIndex)
   , mIsAnimValItem(aIsAnimValItem)
 {
   // These shifts are in sync with the members in the header.
   NS_ABORT_IF_FALSE(aList &&
-                    aListIndex < (1U << 31), "bad arg");
+                    aListIndex <= MaxListIndex(), "bad arg");
 
   NS_ABORT_IF_FALSE(IndexIsValid(), "Bad index for DOMSVGPathSeg!");
 }
 
 DOMSVGPathSeg::DOMSVGPathSeg()
   : mList(nsnull)
   , mListIndex(0)
   , mIsAnimValItem(PR_FALSE)
--- a/content/svg/content/src/DOMSVGPathSeg.h
+++ b/content/svg/content/src/DOMSVGPathSeg.h
@@ -48,16 +48,18 @@ class nsSVGElement;
 // We make DOMSVGPathSeg a pseudo-interface to allow us to QI to it in order to
 // check that the objects that scripts pass to DOMSVGPathSegList methods are
 // our *native* path seg objects.
 //
 // {494A7566-DC26-40C8-9122-52ABD76870C4}
 #define MOZILLA_DOMSVGPATHSEG_IID \
   { 0x494A7566, 0xDC26, 0x40C8, { 0x91, 0x22, 0x52, 0xAB, 0xD7, 0x68, 0x70, 0xC4 } }
 
+#define MOZ_SVG_LIST_INDEX_BIT_COUNT 31
+
 namespace mozilla {
 
 /**
  * Class DOMSVGPathSeg
  *
  * This class is the base class of the classes that create the DOM objects that
  * wrap the internal path segments that are encoded in an SVGPathData. Its
  * sub-classes are also used to create the objects returned by
@@ -116,16 +118,20 @@ public:
    * That's not to say that script can't move these DOM objects between
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGPathSegList).)
    */
   void InsertingIntoList(DOMSVGPathSegList *aList,
                          PRUint32 aListIndex,
                          PRBool aIsAnimValItem);
 
+  static PRUint32 MaxListIndex() {
+    return (1U << MOZ_SVG_LIST_INDEX_BIT_COUNT) - 1;
+  }
+
   /// This method is called to notify this object that its list index changed.
   void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
@@ -193,17 +199,17 @@ protected:
   PRBool IndexIsValid();
 #endif
 
   nsRefPtr<DOMSVGPathSegList> mList;
 
   // Bounds for the following are checked in the ctor, so be sure to update
   // that if you change the capacity of any of the following.
 
-  PRUint32 mListIndex:31;
+  PRUint32 mListIndex:MOZ_SVG_LIST_INDEX_BIT_COUNT;
   PRUint32 mIsAnimValItem:1; // PRUint32 because MSVC won't pack otherwise
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(DOMSVGPathSeg, MOZILLA_DOMSVGPATHSEG_IID)
 
 } // namespace mozilla
 
 nsIDOMSVGPathSeg*
@@ -270,9 +276,11 @@ NS_NewSVGPathSegCurvetoCubicSmoothRel(fl
                                       float x2, float y2);
 
 nsIDOMSVGPathSeg*
 NS_NewSVGPathSegCurvetoQuadraticSmoothAbs(float x, float y);
 
 nsIDOMSVGPathSeg*
 NS_NewSVGPathSegCurvetoQuadraticSmoothRel(float x, float y);
 
+#undef MOZ_SVG_LIST_INDEX_BIT_COUNT
+
 #endif // MOZILLA_DOMSVGPATHSEG_H__
--- a/content/svg/content/src/DOMSVGPathSegList.cpp
+++ b/content/svg/content/src/DOMSVGPathSegList.cpp
@@ -170,16 +170,22 @@ DOMSVGPathSegList::InternalListWillChang
     // Only now may we truncate mItems
     mItems.SetLength(newLength);
 
   } else if (dataIndex < dataLength) {
     // aNewValue has more items than our previous internal counterpart
 
     // Sync mItems:
     while (dataIndex < dataLength) {
+      if (mItems.Length() &&
+          mItems.Length() - 1 > DOMSVGPathSeg::MaxListIndex()) {
+        // It's safe to get out of sync with our internal list as long as we
+        // have FEWER items than it does.
+        return;
+      }
       if (!mItems.AppendElement(ItemProxy(nsnull, dataIndex))) {
         // OOM
         Clear();
         return;
       }
       dataIndex += 1 + SVGPathSegUtils::ArgCountForType(SVGPathSegUtils::DecodeType(aNewValue.mData[dataIndex]));
     }
   }
@@ -309,30 +315,35 @@ DOMSVGPathSegList::InsertItemBefore(nsID
                                     PRUint32 aIndex,
                                     nsIDOMSVGPathSeg **_retval)
 {
   *_retval = nsnull;
   if (IsAnimValList()) {
     return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
   }
 
+  PRUint32 internalIndex;
+  if (aIndex < Length()) {
+    internalIndex = mItems[aIndex].mInternalDataIndex;
+  } else {
+    aIndex = Length();
+    internalIndex = InternalList().mData.Length();
+  }
+  if (aIndex >= DOMSVGPathSeg::MaxListIndex()) {
+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
+  }
+
   nsCOMPtr<DOMSVGPathSeg> domItem = do_QueryInterface(aNewItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
   if (domItem->HasOwner()) {
     domItem = domItem->Clone(); // must do this before changing anything!
   }
-  PRUint32 internalIndex;
-  if (aIndex < Length()) {
-    internalIndex = mItems[aIndex].mInternalDataIndex;
-  } else {
-    aIndex = Length();
-    internalIndex = InternalList().mData.Length();
-  }
+
   PRUint32 argCount = SVGPathSegUtils::ArgCountForType(domItem->Type());
 
   // Ensure we have enough memory so we can avoid complex error handling below:
   if (!mItems.SetCapacity(mItems.Length() + 1) ||
       !InternalList().mData.SetCapacity(InternalList().mData.Length() + 1 + argCount)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
--- a/content/svg/content/src/DOMSVGPoint.h
+++ b/content/svg/content/src/DOMSVGPoint.h
@@ -50,16 +50,18 @@ class nsSVGElement;
 // check that the objects that scripts pass to DOMSVGPointList methods are
 // our *native* point objects.
 //
 // {d6b6c440-af8d-40ee-856b-02a317cab275}
 #define MOZILLA_DOMSVGPOINT_IID \
   { 0xd6b6c440, 0xaf8d, 0x40ee, \
     { 0x85, 0x6b, 0x02, 0xa3, 0x17, 0xca, 0xb2, 0x75 } }
 
+#define MOZ_SVG_LIST_INDEX_BIT_COUNT 30
+
 namespace mozilla {
 
 /**
  * Class DOMSVGPoint
  *
  * This class creates the DOM objects that wrap internal SVGPoint objects that
  * are in an SVGPointList. It is also used to create the objects returned by
  * SVGSVGElement.createSVGPoint() and other functions that return DOM SVGPoint
@@ -87,17 +89,17 @@ public:
               PRBool aIsAnimValItem)
     : mList(aList)
     , mListIndex(aListIndex)
     , mIsReadonly(PR_FALSE)
     , mIsAnimValItem(aIsAnimValItem)
   {
     // These shifts are in sync with the members.
     NS_ABORT_IF_FALSE(aList &&
-                      aListIndex < (1U << 30), "bad arg");
+                      aListIndex <= MaxListIndex(), "bad arg");
 
     NS_ABORT_IF_FALSE(IndexIsValid(), "Bad index for DOMSVGPoint!");
   }
 
   DOMSVGPoint(const DOMSVGPoint *aPt = nsnull)
     : mList(nsnull)
     , mListIndex(0)
     , mIsReadonly(PR_FALSE)
@@ -170,16 +172,20 @@ public:
    * That's not to say that script can't move these DOM objects between
    * lists - it can - it's just that the logic to handle that (and send out
    * the necessary notifications) is located elsewhere (in DOMSVGPointList).)
    */
   void InsertingIntoList(DOMSVGPointList *aList,
                          PRUint32 aListIndex,
                          PRBool aIsAnimValItem);
 
+  static PRUint32 MaxListIndex() {
+    return (1U << MOZ_SVG_LIST_INDEX_BIT_COUNT) - 1;
+  }
+
   /// This method is called to notify this object that its list index changed.
   void UpdateListIndex(PRUint32 aListIndex) {
     mListIndex = aListIndex;
   }
 
   /**
    * This method is called to notify this DOM object that it is about to be
    * removed from its current DOM list so that it can first make a copy of its
@@ -220,21 +226,23 @@ protected:
   PRBool IndexIsValid();
 #endif
 
   nsRefPtr<DOMSVGPointList> mList;
 
   // Bounds for the following are checked in the ctor, so be sure to update
   // that if you change the capacity of any of the following.
 
-  PRUint32 mListIndex:30;
+  PRUint32 mListIndex:MOZ_SVG_LIST_INDEX_BIT_COUNT;
   PRUint32 mIsReadonly:1;    // PRUint32 because MSVC won't pack otherwise
   PRUint32 mIsAnimValItem:1; // PRUint32 because MSVC won't pack otherwise
 
   // The following member is only used when we're not in a list:
   SVGPoint mPt;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(DOMSVGPoint, MOZILLA_DOMSVGPOINT_IID)
 
 } // namespace mozilla
 
+#undef MOZ_SVG_LIST_INDEX_BIT_COUNT
+
 #endif // MOZILLA_DOMSVGPOINT_H__
--- a/content/svg/content/src/DOMSVGPointList.cpp
+++ b/content/svg/content/src/DOMSVGPointList.cpp
@@ -97,17 +97,23 @@ DOMSVGPointList::~DOMSVGPointList()
 void
 DOMSVGPointList::InternalListWillChangeTo(const SVGPointList& aNewValue)
 {
   // When the number of items in our internal counterpart changes, we MUST stay
   // in sync. Everything in the scary comment in
   // DOMSVGLengthList::InternalBaseValListWillChangeTo applies here too!
 
   PRUint32 oldLength = mItems.Length();
+
   PRUint32 newLength = aNewValue.Length();
+  if (newLength > DOMSVGPoint::MaxListIndex()) {
+    // It's safe to get out of sync with our internal list as long as we have
+    // FEWER items than it does.
+    newLength = DOMSVGPoint::MaxListIndex();
+  }
 
   // If our length will decrease, notify the items that will be removed:
   for (PRUint32 i = newLength; i < oldLength; ++i) {
     if (mItems[i]) {
       mItems[i]->RemovingFromList();
     }
   }
 
@@ -245,24 +251,28 @@ DOMSVGPointList::InsertItemBefore(nsIDOM
                                   PRUint32 aIndex,
                                   nsIDOMSVGPoint **_retval)
 {
   *_retval = nsnull;
   if (IsAnimValList()) {
     return NS_ERROR_DOM_NO_MODIFICATION_ALLOWED_ERR;
   }
 
+  aIndex = NS_MIN(aIndex, Length());
+  if (aIndex >= DOMSVGPoint::MaxListIndex()) {
+    return NS_ERROR_DOM_INDEX_SIZE_ERR;
+  }
+
   nsCOMPtr<DOMSVGPoint> domItem = do_QueryInterface(aNewItem);
   if (!domItem) {
     return NS_ERROR_DOM_SVG_WRONG_TYPE_ERR;
   }
   if (domItem->HasOwner() || domItem->IsReadonly()) {
     domItem = domItem->Clone(); // must do this before changing anything!
   }
-  aIndex = NS_MIN(aIndex, mItems.Length());
 
   // Ensure we have enough memory so we can avoid complex error handling below:
   if (!mItems.SetCapacity(mItems.Length() + 1) ||
       !InternalList().SetCapacity(InternalList().Length() + 1)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   InternalList().InsertItem(aIndex, domItem->ToSVGPoint());