Dynamic salt formats: Drop a redundant and very confusing pointer copy

Similar (yet different, and relevant) code is used in other formats where
we don't copy just a pointer but a complete salt.

src/7z_common_plug.c

@@ -253,16 +253,11 @@ int sevenzip_valid(char *ciphertext, struct fmt_main *self)
 void *sevenzip_get_salt(char *ciphertext)
 {
 	sevenzip_salt_t cs;
-	sevenzip_salt_t *psalt;
-	static void *ptr;
 	char *ctcopy = xstrdup(ciphertext);
 	char *keeptr = ctcopy;
 	int i;
 	char *p;
 
-	if (!ptr)
-		ptr = mem_alloc_tiny(sizeof(sevenzip_salt_t*),
-		                     sizeof(sevenzip_salt_t*));
 	memset(&cs, 0, sizeof(cs));
 	ctcopy += TAG_LENGTH;
 	p = strtokm(ctcopy, "$");
@@ -285,6 +280,9 @@ void *sevenzip_get_salt(char *ciphertext)
 		cs.crc = atou(p); /* unsigned function */
 	p = strtokm(NULL, "$");
 	cs.aes_length = atoll(p);
+
+	/* Now we know the size of the dyna salt, so we can allocate */
+	static sevenzip_salt_t *psalt;
 	psalt = mem_alloc(sizeof(sevenzip_salt_t) + cs.aes_length - 1);
 	memcpy(psalt, &cs, sizeof(cs));
 	p = strtokm(NULL, "$");
@@ -314,8 +312,7 @@ void *sevenzip_get_salt(char *ciphertext)
 	psalt->dsalt.salt_cmp_size = SALT_CMP_SIZE(sevenzip_salt_t, aes_length, data, psalt->aes_length);
 	psalt->dsalt.salt_alloc_needs_free = 1;
 
-	memcpy(ptr, &psalt, sizeof(void*));
-	return ptr;
+	return &psalt;
 }
 
 int sevenzip_salt_compare(const void *x, const void *y)

src/gpg_common_plug.c

@@ -859,11 +859,9 @@ void *gpg_common_get_salt(char *ciphertext)
 	char *keeptr = ctcopy;
 	int i;
 	char *p;
-	struct gpg_common_custom_salt cs, *psalt;
-	static unsigned char *ptr;
+	struct gpg_common_custom_salt cs;
 
 	memset(&cs, 0, sizeof(cs));
-	if (!ptr) ptr = mem_alloc_tiny(sizeof(struct gpg_common_custom_salt*),sizeof(struct gpg_common_custom_salt*));
 	ctcopy += FORMAT_TAG_LEN;	/* skip over "$gpg$" marker and first '*' */
 	p = strtokm(ctcopy, "*");
 	cs.pk_algorithm = atoi(p);
@@ -875,6 +873,7 @@ void *gpg_common_get_salt(char *ciphertext)
 
 	/* Ok, now we 'know' the size of the dyna salt, so we can allocate */
 	/* note the +64 is due to certain algo's reading dirty data, up to 64 bytes past end */
+	static struct gpg_common_custom_salt *psalt;
 	psalt = mem_calloc(sizeof(struct gpg_common_custom_salt) + cs.datalen + 64, 1);
 	psalt->pk_algorithm = cs.pk_algorithm;
 	psalt->symmetric_mode = cs.symmetric_mode;
@@ -1047,8 +1046,7 @@ void *gpg_common_get_salt(char *ciphertext)
 	psalt->dsalt.salt_cmp_offset = SALT_CMP_OFF(struct gpg_common_custom_salt, datalen);
 	psalt->dsalt.salt_cmp_size = SALT_CMP_SIZE(struct gpg_common_custom_salt, datalen, data, psalt->datalen);
 
-	memcpy(ptr, &psalt, sizeof(struct gpg_common_custom_salt*));
-	return (void*)ptr;
+	return &psalt;
 }
 
 static unsigned int length_of_multi_precision_integer(const unsigned char *buf)

src/krb5_tgs_common_plug.c

@@ -119,69 +119,61 @@ int krb5tgs_valid(char *ciphertext, struct fmt_main *self)
 void *krb5tgs_get_salt(char *ciphertext)
 {
 	int i;
-	krb5tgs_salt *psalt;
-	static unsigned char *ptr;
 	char *p;
 	char *ctcopy;
 	char *keeptr;
 
-	if (!ptr)
-		ptr = mem_alloc_tiny(sizeof(krb5tgs_salt*),sizeof(krb5tgs_salt*));
-
 	ctcopy = xstrdup(ciphertext);
 	keeptr = ctcopy;
 
-	if (strncmp(ciphertext, FORMAT_TAG, FORMAT_TAG_LEN) == 0) {
-		ctcopy += FORMAT_TAG_LEN;
-		if (ctcopy[0] == '*') {
-			ctcopy++;
-			p = strtokm(ctcopy, "*");
-			ctcopy += strlen(p) + 2;
-			goto edata;
-		}
-		if (ctcopy[0]=='$')
-			ctcopy++;
+	ctcopy += FORMAT_TAG_LEN;
+	if (ctcopy[0] == '*') {
+		ctcopy++;
+		p = strtokm(ctcopy, "*");
+		ctcopy += strlen(p) + 2;
+		goto edata;
 	}
+	if (ctcopy[0]=='$')
+		ctcopy++;
 
 edata:
-	if (((p = strtokm(ctcopy, "$")) != NULL) && strlen(p) == 32) {	/* assume checksum */
-
-		unsigned char edata1[16];
-		for (i = 0; i < 16; i++) {
-			edata1[i] =
-				atoi16[ARCH_INDEX(p[i * 2])] * 16 +
-				atoi16[ARCH_INDEX(p[i * 2 + 1])];
-		}
-
-		/* skip '$' */
-		p += strlen(p) + 1;
-
-		/* retrieve non-constant length of edata2 */
-		for (i = 0; p[i] != '\0'; i++)
-			;
-
-		size_t edata2len = i / 2;
-		if (edata2len > krb5tgs_max_data_len)
-			krb5tgs_max_data_len = edata2len;
-
-		psalt = mem_calloc(1, sizeof(krb5tgs_salt) + edata2len);
-		memcpy(psalt->edata1, edata1, 16);
-		psalt->edata2len = edata2len;
-
-		for (i = 0; i < edata2len; i++) {	/* assume edata2 */
-			psalt->edata2[i] =
-				atoi16[ARCH_INDEX(p[i * 2])] * 16 +
-				atoi16[ARCH_INDEX(p[i * 2 + 1])];
-		}
-
-		/* Set the JtR core linkage stuff for this dyna_salt */
-		psalt->dsalt.salt_cmp_offset = SALT_CMP_OFF(krb5tgs_salt, edata1);
-		psalt->dsalt.salt_cmp_size = SALT_CMP_SIZE(krb5tgs_salt, edata1, edata2len, psalt->edata2len);
-		psalt->dsalt.salt_alloc_needs_free = 1;
-
-		memcpy(ptr, &psalt, sizeof(krb5tgs_salt*));
+	p = strtokm(ctcopy, "$");
+
+	unsigned char edata1[16];
+	for (i = 0; i < 16; i++) {
+		edata1[i] =
+			atoi16[ARCH_INDEX(p[i * 2])] * 16 +
+			atoi16[ARCH_INDEX(p[i * 2 + 1])];
+	}
+
+	/* skip '$' */
+	p += strlen(p) + 1;
+
+	/* retrieve non-constant length of edata2 */
+	for (i = 0; p[i] != '\0'; i++)
+		;
+
+	size_t edata2len = i / 2;
+	if (edata2len > krb5tgs_max_data_len)
+		krb5tgs_max_data_len = edata2len;
+
+	/* Now build dynamic salt and return a pointer to a pointer to it */
+	static krb5tgs_salt *psalt;
+	psalt = mem_calloc(1, sizeof(krb5tgs_salt) + edata2len);
+	memcpy(psalt->edata1, edata1, 16);
+	psalt->edata2len = edata2len;
+
+	for (i = 0; i < edata2len; i++) {	/* assume edata2 */
+		psalt->edata2[i] =
+			atoi16[ARCH_INDEX(p[i * 2])] * 16 +
+			atoi16[ARCH_INDEX(p[i * 2 + 1])];
 	}
 	MEM_FREE(keeptr);
 
-	return (void*)ptr;
+	/* Set the JtR core linkage stuff for this dyna_salt */
+	psalt->dsalt.salt_cmp_offset = SALT_CMP_OFF(krb5tgs_salt, edata1);
+	psalt->dsalt.salt_cmp_size = SALT_CMP_SIZE(krb5tgs_salt, edata1, edata2len, psalt->edata2len);
+	psalt->dsalt.salt_alloc_needs_free = 1;
+
+	return &psalt;
 }

src/luks_fmt_plug.c

@@ -463,14 +463,11 @@ static void *get_salt(char *ciphertext)
 	int cnt;
 	unsigned char *out;
 	unsigned char *buf;
-	struct custom_salt_LUKS cs, *psalt;
-	static unsigned char *ptr;
+	struct custom_salt_LUKS cs;
 	size_t size = 0;
 
 	ctcopy += FORMAT_TAG_LEN;
 
-
-	if (!ptr) ptr = mem_alloc_tiny(sizeof(struct custom_salt*),sizeof(struct custom_salt*));
 	memset(&cs, 0, sizeof(cs));
 	cs.bestiter = INT_MAX;
 	out = (unsigned char*)&cs.myphdr;
@@ -514,7 +511,9 @@ static void *get_salt(char *ciphertext)
 			john_ntohl(cs.myphdr.keyblock[cs.bestslot].stripes));
 	MEM_FREE(keeptr);
 
-	psalt = (struct custom_salt_LUKS*)mem_alloc_tiny(sizeof(struct custom_salt_LUKS)+size, 4);
+	/* Now build dynamic salt and return a pointer to a pointer to it */
+	static struct custom_salt_LUKS *psalt;
+	psalt = mem_alloc_tiny(sizeof(struct custom_salt_LUKS)+size, 4);
 	memcpy(psalt, &cs, sizeof(cs));
 	memcpy(psalt->cipherbuf, buf, size);
 	MEM_FREE(buf);
@@ -524,8 +523,7 @@ static void *get_salt(char *ciphertext)
 	psalt->dsalt.salt_cmp_offset = SALT_CMP_OFF(struct custom_salt_LUKS, myphdr);
 	psalt->dsalt.salt_cmp_size = SALT_CMP_SIZE(struct custom_salt_LUKS, myphdr, cipherbuf, size);
 
-	memcpy(ptr, &psalt, sizeof(struct custom_salt*));
-	return (void*)ptr;
+	return &psalt;
 }
 
 static void *get_binary(char *ciphertext)

src/pkzip.c

@@ -188,14 +188,10 @@ char *winzip_common_split(char *ciphertext, int index, struct fmt_main *self)
 void *winzip_common_get_salt(char *ciphertext)
 {
 	uint64_t i;
-	winzip_salt salt, *psalt;
-	static unsigned char *ptr;
+	winzip_salt salt;
 	c8 *copy_mem = xstrdup(ciphertext);
 	c8 *cp, *p;
 
-	if (!ptr)
-		ptr = mem_alloc_tiny(sizeof(winzip_salt*),sizeof(winzip_salt*));
-
 	p = copy_mem + WINZIP_TAG_LENGTH + 1; /* skip over "$zip2$*" */
 	memset(&salt, 0, sizeof(salt));
 	cp = strtokm(p, "*"); // type
@@ -215,7 +211,8 @@ void *winzip_common_get_salt(char *ciphertext)
 
 	// Ok, now create the allocated salt record we are going to return back to John, using the dynamic
 	// sized data buffer.
-	psalt = (winzip_salt*)mem_calloc(1, sizeof(winzip_salt) + salt.comp_len);
+	static winzip_salt *psalt;
+	psalt = mem_calloc(1, sizeof(winzip_salt) + salt.comp_len);
 	psalt->v.type = salt.v.type;
 	psalt->v.mode = salt.v.mode;
 	psalt->comp_len = salt.comp_len;
@@ -234,8 +231,7 @@ void *winzip_common_get_salt(char *ciphertext)
 
 	MEM_FREE(copy_mem);
 
-	memcpy(ptr, &psalt, sizeof(winzip_salt*));
-	return (void*)ptr;
+	return &psalt;
 }
 
 void *winzip_common_binary(char *ciphertext) {