Semmle: Ensure stack memory is not leaked through uninitialized… (#3561)

New stack memory can contain anything including passwords/private keys.
In these functions we return structs that can have their padding
bytes uninitialized. By first zeroing out the struct fully, we try to
ensure that any data that is in these padding bytes is at least
overwritten once. It might not be zero anymore after setting the fields,
but at least it shouldn't be private data anymore.

(cherry picked from commit c4cc26ed37)

src/backend/distributed/executor/adaptive_executor.c

@@ -135,6 +135,7 @@
 #include "distributed/adaptive_executor.h"
 #include "distributed/cancel_utils.h"
 #include "distributed/citus_custom_scan.h"
+#include "distributed/citus_safe_lib.h"
 #include "distributed/connection_management.h"
 #include "distributed/deparse_shard_query.h"
 #include "distributed/distributed_execution_locks.h"
@@ -982,11 +983,13 @@ static TransactionProperties
 DecideTransactionPropertiesForTaskList(RowModifyLevel modLevel, List *taskList, bool
 									   exludeFromTransaction)
 {
-	TransactionProperties xactProperties = {
+	TransactionProperties xactProperties;
-		.errorOnAnyFailure = false,
+
-		.useRemoteTransactionBlocks = TRANSACTION_BLOCKS_ALLOWED,
+	/* ensure uninitialized padding doesn't escape the function */
-		.requires2PC = false
+	memset_struct_0(xactProperties);
-	};
+	xactProperties.errorOnAnyFailure = false;
+	xactProperties.useRemoteTransactionBlocks = TRANSACTION_BLOCKS_ALLOWED;
+	xactProperties.requires2PC = false;
 
 	if (taskList == NIL)
 	{

src/backend/distributed/metadata/node_metadata.c

@@ -21,6 +21,7 @@
 #include "catalog/namespace.h"
 #include "commands/sequence.h"
 #include "distributed/citus_acquire_lock.h"
+#include "distributed/citus_safe_lib.h"
 #include "distributed/colocation_utils.h"
 #include "distributed/commands.h"
 #include "distributed/commands/utility_hook.h"
@@ -109,11 +110,14 @@ PG_FUNCTION_INFO_V1(get_shard_id_for_distribution_column);
 static NodeMetadata
 DefaultNodeMetadata()
 {
-	NodeMetadata nodeMetadata = {
+	NodeMetadata nodeMetadata;
-		.nodeRack = WORKER_DEFAULT_RACK,
+
-		.shouldHaveShards = true,
+	/* ensure uninitialized padding doesn't escape the function */
-		.groupId = INVALID_GROUP_ID,
+	memset_struct_0(nodeMetadata);
-	};
+	nodeMetadata.nodeRack = WORKER_DEFAULT_RACK;
+	nodeMetadata.shouldHaveShards = true;
+	nodeMetadata.groupId = INVALID_GROUP_ID;
+
 	return nodeMetadata;
 }
 

src/include/distributed/citus_safe_lib.h

@@ -26,4 +26,6 @@ void * SafeBsearch(const void *key, const void *ptr, rsize_t count, rsize_t size
 				   int (*comp)(const void *, const void *));
 int SafeSnprintf(char *str, rsize_t count, const char *fmt, ...);
 
+#define memset_struct_0(variable) memset(&variable, 0, sizeof(variable))
+
 #endif

src/include/distributed/version_compat.h

@@ -12,8 +12,10 @@
 #define VERSION_COMPAT_H
 
 #include "postgres.h"
+
 #include "commands/explain.h"
 #include "catalog/namespace.h"
+#include "distributed/citus_safe_lib.h"
 #include "nodes/parsenodes.h"
 #include "parser/parse_func.h"
 #if (PG_VERSION_NUM >= 120000)
@@ -71,10 +73,12 @@ FileReadCompat(FileCompat *file, char *buffer, int amount, uint32 wait_event_inf
 static inline FileCompat
 FileCompatFromFileStart(File fileDesc)
 {
-	FileCompat fc = {
+	FileCompat fc;
-		.fd = fileDesc,
+
-		.offset = 0
+	/* ensure uninitialized padding doesn't escape the function */
-	};
+	memset_struct_0(fc);
+	fc.fd = fileDesc;
+	fc.offset = 0;
 
 	return fc;
 }