[Bf-blender-cvs] [2561145da8d] master: Fix T91096: VSE use after free with overwrite enabled

[Richard Antalik]

Commit: 2561145da8d1e6db6617fa67c5a306d6e07e34e5
Author: Richard Antalik
Date:   Sat Oct 9 10:11:10 2021 +0200
Branches: master
https://developer.blender.org/rB2561145da8d1e6db6617fa67c5a306d6e07e34e5

Fix T91096: VSE use after free with overwrite enabled

Strip was flagged for deletion in `seq_transform_handle_overwrite()`
on `STRIP_OVERLAP_IS_FULL`. Then it is removed in
`SEQ_edit_strip_split()` before it should be.

Handle `STRIP_OVERLAP_IS_FULL` in separate loop.

This may not be complete solution, because in example file overlap is
caused between 2 transformed strips and one that is "static".
Such operation should not be possible in first place. This fixes the
crash at lest, so improvement in behavior can be handled separately.

Differential Revision: https://developer.blender.org/D12751

===================================================================

M	source/blender/editors/transform/transform_convert_sequencer.c

===================================================================

diff --git a/source/blender/editors/transform/transform_convert_sequencer.c b/source/blender/editors/transform/transform_convert_sequencer.c
index bf320595d6c..70089164d8a 100644
--- a/source/blender/editors/transform/transform_convert_sequencer.c
+++ b/source/blender/editors/transform/transform_convert_sequencer.c
@@ -458,6 +458,7 @@ static void seq_transform_handle_overwrite_split(const TransInfo *t,
   SEQ_edit_strip_split(
       bmain, scene, seqbase, split_strip, transformed->enddisp, SEQ_SPLIT_SOFT, NULL);
   SEQ_edit_flag_for_removal(scene, seqbase_active_get(t), split_strip);
+  SEQ_edit_remove_flagged_sequences(t->scene, seqbase_active_get(t));
 }
 
 /* Trim strips by adjusting handle position.
@@ -498,8 +499,8 @@ static void seq_transform_handle_overwrite_trim(const TransInfo *t,
 static void seq_transform_handle_overwrite(const TransInfo *t, SeqCollection *transformed_strips)
 {
   SeqCollection *targets = query_overwrite_targets(t, transformed_strips);
+  SeqCollection *strips_to_delete = SEQ_collection_create(__func__);
 
-  bool strips_delete = false;
   Sequence *target;
   Sequence *transformed;
   SEQ_ITERATOR_FOREACH (target, targets) {
@@ -511,13 +512,10 @@ static void seq_transform_handle_overwrite(const TransInfo *t, SeqCollection *tr
       const eOvelapDescrition overlap = overlap_description_get(transformed, target);
 
       if (overlap == STRIP_OVERLAP_IS_FULL) {
-        /* Remove covered strip. */
-        SEQ_edit_flag_for_removal(t->scene, seqbase_active_get(t), target);
-        strips_delete = true;
+        SEQ_collection_append_strip(target, strips_to_delete);
       }
       else if (overlap == STRIP_OVERLAP_IS_INSIDE) {
         seq_transform_handle_overwrite_split(t, transformed, target);
-        strips_delete = true;
       }
       else if (ELEM(overlap, STRIP_OVERLAP_LEFT_SIDE, STRIP_OVERLAP_RIGHT_SIDE)) {
         seq_transform_handle_overwrite_trim(t, transformed, target, overlap);
@@ -527,9 +525,16 @@ static void seq_transform_handle_overwrite(const TransInfo *t, SeqCollection *tr
 
   SEQ_collection_free(targets);
 
-  if (strips_delete) {
+  /* Remove covered strips. This must be done in separate loop, because `SEQ_edit_strip_split()`
+   * also uses `SEQ_edit_remove_flagged_sequences()`. See T91096. */
+  if (SEQ_collection_len(strips_to_delete) > 0) {
+    Sequence *seq;
+    SEQ_ITERATOR_FOREACH (seq, strips_to_delete) {
+      SEQ_edit_flag_for_removal(t->scene, seqbase_active_get(t), seq);
+    }
     SEQ_edit_remove_flagged_sequences(t->scene, seqbase_active_get(t));
   }
+  SEQ_collection_free(strips_to_delete);
 }
 
 static void seq_transform_handle_overlap_shuffle(const TransInfo *t,