Ensure all fields in cluster-objects structs are default-initialized. (…

…#13432) Otherwise we can end up reading random values from un-initialized memory (e.g. if a mandatory command/struct/event field is not sent by the other side). Fixes #10271
project-chip · Feb 23, 2022 · 4379747 · 4379747
1 parent 07807e9
commit 4379747
Show file tree

Hide file tree

Showing 6 changed files with 2,417 additions and 2,349 deletions.
diff --git a/src/app/clusters/network-commissioning/network-commissioning.cpp b/src/app/clusters/network-commissioning/network-commissioning.cpp
@@ -262,8 +262,6 @@ void Instance::HandleRemoveNetwork(HandlerContext & ctx, const Commands::RemoveN
 
 void Instance::HandleConnectNetwork(HandlerContext & ctx, const Commands::ConnectNetwork::DecodableType & req)
 {
-    Commands::ConnectNetworkResponse::Type response;
-
     mAsyncCommandHandle = app::CommandHandler::Handle(&ctx.mCommandHandler);
     mpWirelessDriver->ConnectNetwork(req.networkID, this);
 }

diff --git a/src/app/common/templates/templates.json b/src/app/common/templates/templates.json
@@ -17,6 +17,10 @@
         {
             "name": "cluster_objects_struct",
             "path": "../../zap-templates/partials/cluster-objects-struct.zapt"
+        },
+        {
+            "name": "cluster_objects_field_init",
+            "path": "../../zap-templates/partials/cluster-objects-field-init.zapt"
         }
     ],
     "templates": [

diff --git a/src/app/data-model/DecodableList.h b/src/app/data-model/DecodableList.h
@@ -38,6 +38,12 @@ template <typename T>
 class DecodableList
 {
 public:
+    DecodableList()
+    {
+        // Init to an empty list.
+        mReader.Init(nullptr, 0);
+    }
+
     /*
      * @brief
      *

diff --git a/src/app/zap-templates/partials/cluster-objects-field-init.zapt b/src/app/zap-templates/partials/cluster-objects-field-init.zapt
@@ -0,0 +1,17 @@
+{{! For now, just initialize fields to type defaults.  Longer-term, we
+    may want to use spec-defined default values here. }}
+{{#unless isOptional}} {{! Optionals inited by constructor }}
+  {{~#unless isNullable}} {{! Nullables inited by constructor }}
+    {{~#unless isArray}} {{! DataModel lists inited by constructor }}
+      {{~#unless entryType}} {{! DataModel lists (for attributes) inited by constructor }}
+        {{~#unless (isString type)}} {{! Strings are Spans, inited by constructor }}
+          {{~#if_is_struct type}}
+            {{! Structs have their own initializers }}
+          {{~else~}}
+          = static_cast<{{zapTypeToEncodableClusterObjectType type}}>(0)
+          {{~/if_is_struct}}
+        {{~/unless}}
+      {{~/unless}}
+    {{~/unless}}
+  {{~/unless}}
+{{~/unless}}
diff --git a/src/app/zap-templates/templates/app/cluster-objects.zapt b/src/app/zap-templates/templates/app/cluster-objects.zapt
@@ -111,7 +111,7 @@ public:
     static constexpr ClusterId GetClusterId() { return Clusters::{{asUpperCamelCase parent.name}}::Id; }
 
     {{#zcl_command_arguments}}
-    {{zapTypeToEncodableClusterObjectType type}} {{asLowerCamelCase label}};
+    {{zapTypeToEncodableClusterObjectType type}} {{asLowerCamelCase label}}{{> cluster_objects_field_init}};
     {{/zcl_command_arguments}}
 
     CHIP_ERROR Encode(TLV::TLVWriter &writer, TLV::Tag tag) const;
@@ -132,7 +132,7 @@ public:
     static constexpr ClusterId GetClusterId() { return Clusters::{{asUpperCamelCase parent.name}}::Id; }
 
     {{#zcl_command_arguments}}
-    {{zapTypeToDecodableClusterObjectType type}} {{asLowerCamelCase label}};
+    {{zapTypeToDecodableClusterObjectType type}} {{asLowerCamelCase label}}{{> cluster_objects_field_init}};
     {{/zcl_command_arguments}}
     CHIP_ERROR Decode(TLV::TLVReader &reader);
 };
@@ -174,7 +174,9 @@ struct TypeInfo
         CHIP_ERROR Decode(TLV::TLVReader &reader, const ConcreteAttributePath &path);
 
 {{#zcl_attributes_server}}
-        Attributes::{{asUpperCamelCase label}}::TypeInfo::DecodableType {{asLowerCamelCase label}};
+        {{! isOptional=false because optional attributes don't get represented
+            as Optional types here. }}
+        Attributes::{{asUpperCamelCase label}}::TypeInfo::DecodableType {{asLowerCamelCase label}}{{> cluster_objects_field_init isOptional=false}};
 {{/zcl_attributes_server}}
     };
 };
@@ -201,7 +203,7 @@ public:
     static constexpr ClusterId GetClusterId() { return Clusters::{{asUpperCamelCase parent.name}}::Id; }
 
     {{#zcl_event_fields}}
-    {{zapTypeToEncodableClusterObjectType type}} {{asLowerCamelCase name}};
+    {{zapTypeToEncodableClusterObjectType type}} {{asLowerCamelCase name}}{{> cluster_objects_field_init}};
     {{/zcl_event_fields}}
 
     CHIP_ERROR Encode(TLV::TLVWriter &writer, TLV::Tag tag) const;
@@ -214,7 +216,7 @@ public:
     static constexpr ClusterId GetClusterId() { return Clusters::{{asUpperCamelCase parent.name}}::Id; }
 
     {{#zcl_event_fields}}
-    {{zapTypeToDecodableClusterObjectType type}} {{asLowerCamelCase name}};
+    {{zapTypeToDecodableClusterObjectType type}} {{asLowerCamelCase name}}{{> cluster_objects_field_init}};
     {{/zcl_event_fields}}
 
     CHIP_ERROR Decode(TLV::TLVReader &reader);