Fix T56108: Crash editing corrupted vertex groups

While the file in this report had corrupted values,
this is avoidable without adding any extra overhead.

Use unsigned vertex group indices since we don't need negative values,
this is an alternative to checking they aren't negative in many places.

Vertex group values over INT_MAX is still considered invalid,
so any accidental unsigned wrapping won't be silently ignored.

7 files changed, 19 insertions, 20 deletions

diff --git a/source/blender/blenkernel/BKE_deform.h b/source/blender/blenkernel/BKE_deform.h
index ca06716599c..2911002b9e9 100644
--- a/source/blender/blenkernel/BKE_deform.h
+++ b/source/blender/blenkernel/BKE_deform.h
@@ -106,7 +106,7 @@ void defvert_normalize_subset(struct MDeformVert *dvert,
 void defvert_normalize_lock_single(struct MDeformVert *dvert,
                                    const bool *vgroup_subset,
                                    const int vgroup_tot,
-                                   const int def_nr_lock);
+                                   const uint def_nr_lock);
 void defvert_normalize_lock_map(struct MDeformVert *dvert,
                                 const bool *vgroup_subset,
                                 const int vgroup_tot,
diff --git a/source/blender/blenkernel/intern/armature.c b/source/blender/blenkernel/intern/armature.c
index ed752986ddd..d4498017ffd 100644
--- a/source/blender/blenkernel/intern/armature.c
+++ b/source/blender/blenkernel/intern/armature.c
@@ -1562,8 +1562,8 @@ static void armature_vert_task(void *__restrict userdata,
     int deformed = 0;
     unsigned int j;
     for (j = dvert->totweight; j != 0; j--, dw++) {
-      const int index = dw->def_nr;
-      if (index >= 0 && index < data->defbase_tot && (pchan = data->defnrToPC[index])) {
+      const uint index = dw->def_nr;
+      if (index < data->defbase_tot && (pchan = data->defnrToPC[index])) {
         float weight = dw->weight;
         Bone *bone = pchan->bone;
 
diff --git a/source/blender/blenkernel/intern/deform.c b/source/blender/blenkernel/intern/deform.c
index 79dcdd15bf7..41ff9594cff 100644
--- a/source/blender/blenkernel/intern/deform.c
+++ b/source/blender/blenkernel/intern/deform.c
@@ -255,10 +255,9 @@ void defvert_remap(MDeformVert *dvert, int *map, const int map_len)
   unsigned int i;
   for (i = dvert->totweight; i != 0; i--, dw++) {
     if (dw->def_nr < map_len) {
-      dw->def_nr = map[dw->def_nr];
+      BLI_assert(map[dw->def_nr] >= 0);
 
-      /* just in case */
-      BLI_assert(dw->def_nr >= 0);
+      dw->def_nr = map[dw->def_nr];
     }
   }
 }
@@ -337,7 +336,7 @@ void defvert_normalize(MDeformVert *dvert)
 void defvert_normalize_lock_single(MDeformVert *dvert,
                                    const bool *vgroup_subset,
                                    const int vgroup_tot,
-                                   const int def_nr_lock)
+                                   const uint def_nr_lock)
 {
   if (dvert->totweight == 0) {
     /* nothing */
diff --git a/source/blender/blenkernel/intern/mesh_validate.c b/source/blender/blenkernel/intern/mesh_validate.c
index 4aa5bfa04ab..e4de75b923f 100644
--- a/source/blender/blenkernel/intern/mesh_validate.c
+++ b/source/blender/blenkernel/intern/mesh_validate.c
@@ -784,22 +784,24 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
       for (j = 0, dw = dv->dw; j < dv->totweight; j++, dw++) {
         /* note, greater than max defgroups is accounted for in our code, but not < 0 */
         if (!isfinite(dw->weight)) {
-          PRINT_ERR("\tVertex deform %u, group %d has weight: %f", i, dw->def_nr, dw->weight);
+          PRINT_ERR("\tVertex deform %u, group %u has weight: %f", i, dw->def_nr, dw->weight);
           if (do_fixes) {
             dw->weight = 0.0f;
             fix_flag.verts_weight = true;
           }
         }
         else if (dw->weight < 0.0f || dw->weight > 1.0f) {
-          PRINT_ERR("\tVertex deform %u, group %d has weight: %f", i, dw->def_nr, dw->weight);
+          PRINT_ERR("\tVertex deform %u, group %u has weight: %f", i, dw->def_nr, dw->weight);
           if (do_fixes) {
             CLAMP(dw->weight, 0.0f, 1.0f);
             fix_flag.verts_weight = true;
           }
         }
 
-        if (dw->def_nr < 0) {
-          PRINT_ERR("\tVertex deform %u, has invalid group %d", i, dw->def_nr);
+        /* Not technically incorrect since this is unsigned, however,
+         * a value over INT_MAX is almost certainly caused by wrapping an unsigned int. */
+        if (dw->def_nr >= INT_MAX) {
+          PRINT_ERR("\tVertex deform %u, has invalid group %u", i, dw->def_nr);
           if (do_fixes) {
             defvert_remove_group(dv, dw);
             fix_flag.verts_weight = true;
diff --git a/source/blender/collada/ControllerExporter.cpp b/source/blender/collada/ControllerExporter.cpp
index ef91fcfa62c..0119aba7dfd 100644
--- a/source/blender/collada/ControllerExporter.cpp
+++ b/source/blender/collada/ControllerExporter.cpp
@@ -230,19 +230,17 @@ void ControllerExporter::export_skin_controller(Object *ob, Object *ob_arm)
       float sumw = 0.0f;
 
       for (j = 0; j < vert->totweight; j++) {
-        int idx = vert->dw[j].def_nr;
+        uint idx = vert->dw[j].def_nr;
         if (idx >= joint_index_by_def_index.size()) {
           /* XXX: Maybe better find out where and
            *      why the Out Of Bound indexes get created ? */
           oob_counter += 1;
         }
         else {
-          if (idx >= 0) {
-            int joint_index = joint_index_by_def_index[idx];
-            if (joint_index != -1 && vert->dw[j].weight > 0.0f) {
-              jw[joint_index] += vert->dw[j].weight;
-              sumw += vert->dw[j].weight;
-            }
+          int joint_index = joint_index_by_def_index[idx];
+          if (joint_index != -1 && vert->dw[j].weight > 0.0f) {
+            jw[joint_index] += vert->dw[j].weight;
+            sumw += vert->dw[j].weight;
           }
         }
       }
diff --git a/source/blender/makesdna/DNA_meshdata_types.h b/source/blender/makesdna/DNA_meshdata_types.h
index 648389d610f..be904c5bf94 100644
--- a/source/blender/makesdna/DNA_meshdata_types.h
+++ b/source/blender/makesdna/DNA_meshdata_types.h
@@ -280,7 +280,7 @@ typedef struct MStringProperty {
  */
 typedef struct MDeformWeight {
   /** The index for the vertex group, must *always* be unique when in an array. */
-  int def_nr;
+  unsigned int def_nr;
   /** Weight between 0.0 and 1.0. */
   float weight;
 } MDeformWeight;
diff --git a/source/blender/modifiers/intern/MOD_mask.c b/source/blender/modifiers/intern/MOD_mask.c
index 8d0f6825ee0..00b0068bd11 100644
--- a/source/blender/modifiers/intern/MOD_mask.c
+++ b/source/blender/modifiers/intern/MOD_mask.c
@@ -131,7 +131,7 @@ static Mesh *applyModifier(ModifierData *md, const ModifierEvalContext *ctx, Mes
     bDeformGroup *def;
     bool *bone_select_array;
     int bone_select_tot = 0;
-    const int defbase_tot = BLI_listbase_count(&ob->defbase);
+    const uint defbase_tot = (uint)BLI_listbase_count(&ob->defbase);
 
     /* check that there is armature object with bones to use, otherwise return original mesh */
     if (ELEM(NULL, oba, oba->pose, ob->defbase.first)) {