コミット: 9d0066d0131365c53752743edc67e4c8b4823c04 - mingw-get (git)

The MinGW.org Installation Manager Tool

コミットメタ情報

• リビジョン: 9d0066d0131365c53752743edc67e4c8b4823c04 (tree)
• 日時: 2013-01-08 22:01:22
• 作者: Keith Marshall <keithmarshall@user...>
• コミッター: Keith Marshall

ログメッセージ

Avoid potential heap corruption in action scheduler.

変更サマリ

• modified: ChangeLog
• modified: src/guiexec.cpp
• modified: src/pkgexec.cpp

差分

--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
[phone]  Keith Marshall  <[e-mail]>
+
+	Avoid potential heap corruption in action scheduler.
+
+	* src/guiexec.cpp (pkgActionItem::Clear): Factor out; relocate to...
+	* src/pkgexec.cpp (pkgActionItem::Clear): ...here; this keeps all use
+	of new and delete operators on the same side of the EXE/DLL boundary
+	as the constructor and destructor for pkgActionItem objects.
+	(pkgActionItem::Schedule): Ensure that all min_wanted and max_wanted
+	references are unique to each allocated pkgActionItem object, so that
+	the destructor may safely release their associated heap memory blocks.
+
 2013-01-07  Keith Marshall  <[e-mail]>
 
 	Update copyright notification for development in new year.

--- a/src/guiexec.cpp
+++ b/src/guiexec.cpp
@@ -4,7 +4,7 @@
  * $Id$
  *
  * Written by Keith Marshall <[e-mail]>
- * Copyright (C) 2012, MinGW.org Project
+ * Copyright (C) 2012, 2013, MinGW.org Project
  *
  *
  * Implementation of XML data loading services for the mingw-get GUI.
@@ -440,54 +440,6 @@ inline unsigned long AppWindowMaker::EnumerateActions( int classified )
   return pkgData->Schedule()->EnumeratePendingActions( classified );
 }
 
-pkgActionItem
-*pkgActionItem::Clear( pkgActionItem *schedule, unsigned long mask )
-#define ACTION_PRESERVE_FAILED (ACTION_DOWNLOAD_FAILED | ACTION_APPLY_FAILED)
-{
-  /* Method to remove those action items which have no attribute flags in common
-   * with the specified mask, from the schedule; return the residual schedule of
-   * items, if any, which were not removed.  (Note that specifying a mask with a
-   * value of 0UL, which is the default, results in removal of all items).
-   */
-  pkgActionItem *residual = NULL;
-
-  /* Starting at the specified item, or the invoking class object item
-   * if no starting point is specified...
-   */
-  if( (schedule != NULL) || ((schedule = this) != NULL) )
-  {
-    /* ...and provided this starting point is not NULL, walk back to
-     * the first item in the associated task schedule...
-     */
-    while( schedule->prev != NULL ) schedule = schedule->prev;
-    while( schedule != NULL )
-    {
-      /* ...then, processing each scheduled task item in sequence, and
-       * keeping track of the next to be processed...
-       */
-      pkgActionItem *nextptr = schedule->next;
-      if( (schedule->flags & mask) == 0 )
-	/*
-	 * ...delete each which doesn't match any masked attribute...
-	 */
-	delete schedule;
-
-      else
-        /* ...otherwise add it to the residual schedule.
-	 */
-	residual = schedule;
-
-      /* In either event, move on to the next item in sequence, if any.
-       */
-      schedule = nextptr;
-    }
-  }
-  /* Ultimately, return a pointer to the last item added to the residual
-   * schedule, or NULL if all items were deleted.
-   */
-  return residual;
-}
-
 static int pkgActionCount( HWND dlg, int id, const char *fmt, int classified )
 {
   /* Helper function to itemise the currently scheduled actions
@@ -609,6 +561,7 @@ int AppWindowMaker::Invoked( void )
 }
 
 long AppWindowMaker::OnCommand( WPARAM cmd )
+#define ACTION_PRESERVE_FAILED (ACTION_DOWNLOAD_FAILED | ACTION_APPLY_FAILED)
 {
   /* Handler for WM_COMMAND messages which are directed to the
    * top level application window.

--- a/src/pkgexec.cpp
+++ b/src/pkgexec.cpp
@@ -4,7 +4,7 @@
  * $Id$
  *
  * Written by Keith Marshall <[e-mail]>
- * Copyright (C) 2009, 2010, 2011, 2012, MinGW.org Project
+ * Copyright (C) 2009, 2010, 2011, 2012, 2013, MinGW.org Project
  *
  *
  * Implementation of package management task scheduler and executive.
@@ -302,7 +302,27 @@ pkgActionItem::Schedule( unsigned long action, pkgActionItem& item )
      * action, in case it is required to complete the request.
      */
     action |= ACTION_DOWNLOAD;
-  rtn->flags = action | (rtn->flags & ~ACTION_MASK);
+  rtn->flags = action | (item.flags & ~ACTION_MASK);
+
+  /* The min_wanted and max_wanted properties, if defined, refer
+   * to dynamically allocated memory blocks, (on the heap); these
+   * must have only one action item owner; currently, the original
+   * item and the copy we've just made are both effective owners,
+   * and we want only the copy to retain this ownership, we must
+   * detach them from the original item.
+   */
+  item.min_wanted = item.max_wanted = NULL;
+
+  /* Similarly, we must transfer any linkage into the schedule of
+   * actions from the original item to the copy.
+   */
+  if( item.prev != NULL ) (item.prev)->next = rtn;
+  if( item.next != NULL ) (item.next)->prev = rtn;
+  item.prev = item.next = NULL;
+
+  /* Finally, we return the copy, leaving the ultimate disposal
+   * of the original to the caller's discretion.
+   */
   return rtn;
 }
 
@@ -642,6 +662,52 @@ void pkgActionItem::Execute( bool with_download )
   }
 }
 
+pkgActionItem *pkgActionItem::Clear( pkgActionItem *schedule, unsigned long mask )
+{
+  /* Method to remove those action items which have no attribute flags in common
+   * with the specified mask, from the schedule; return the residual schedule of
+   * items, if any, which were not removed.  (Note that specifying a mask with a
+   * value of 0UL, which is the default, results in removal of all items).
+   */
+  pkgActionItem *residual = NULL;
+
+  /* Starting at the specified item, or the invoking class object item
+   * if no starting point is specified...
+   */
+  if( (schedule != NULL) || ((schedule = this) != NULL) )
+  {
+    /* ...and provided this starting point is not NULL, walk back to
+     * the first item in the associated task schedule...
+     */
+    while( schedule->prev != NULL ) schedule = schedule->prev;
+    while( schedule != NULL )
+    {
+      /* ...then, processing each scheduled task item in sequence, and
+       * keeping track of the next to be processed...
+       */
+      pkgActionItem *nextptr = schedule->next;
+      if( (schedule->flags & mask) == 0 )
+	/*
+	 * ...delete each which doesn't match any masked attribute...
+	 */
+	delete schedule;
+
+      else
+        /* ...otherwise add it to the residual schedule.
+	 */
+	residual = schedule;
+
+      /* In either event, move on to the next item in sequence, if any.
+       */
+      schedule = nextptr;
+    }
+  }
+  /* Ultimately, return a pointer to the last item added to the residual
+   * schedule, or NULL if all items were deleted.
+   */
+  return residual;
+}
+
 pkgActionItem::~pkgActionItem()
 {
   /* Destructor...