Skip to content

Commit

Permalink
Ensure all fields in cluster-objects structs are default-initialized. (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Oct 24, 2023
1 parent 7bdf8bc commit 5c00c32
Show file tree
Hide file tree
Showing 6 changed files with 2,417 additions and 2,349 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 4 additions & 0 deletions src/app/common/templates/templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
6 changes: 6 additions & 0 deletions src/app/data-model/DecodableList.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ template <typename T>
class DecodableList
{
public:
DecodableList()
{
// Init to an empty list.
mReader.Init(nullptr, 0);
}

/*
* @brief
*
Expand Down
17 changes: 17 additions & 0 deletions src/app/zap-templates/partials/cluster-objects-field-init.zapt
Original file line number Diff line number Diff line change
@@ -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}}
12 changes: 7 additions & 5 deletions src/app/zap-templates/templates/app/cluster-objects.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
};
Expand Down Expand Up @@ -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}}
};
};
Expand All @@ -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;
Expand All @@ -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);
Expand Down
Loading

0 comments on commit 5c00c32

Please sign in to comment.