Merge lp:~tyhicks/ecryptfs/rewrap-passphrase-cleanups into lp:ecryptfs

[Tyler Hicks]

Fixes minor segfault in ecryptfs-rewrap-passphrase, as well as a bit of cleanup.

[Dustin Kirkland  (kirkland)]

Review: Approve
On Jun 27, 2013 2:58 PM, "Tyler Hicks" <email address hidden> wrote:

> Tyler Hicks has proposed merging
> lp:~tyhicks/ecryptfs/rewrap-passphrase-cleanups into lp:ecryptfs.
>
> Requested reviews:
> eCryptfs (ecryptfs)
>
> For more details, see:
>
> https://code.launchpad.net/~tyhicks/ecryptfs/rewrap-passphrase-cleanups/+merge/171912
>
> Fixes minor segfault in ecryptfs-rewrap-passphrase, as well as a bit of
> cleanup.
> --
>
> https://code.launchpad.net/~tyhicks/ecryptfs/rewrap-passphrase-cleanups/+merge/171912
> Your team eCryptfs is requested to review the proposed merge of
> lp:~tyhicks/ecryptfs/rewrap-passphrase-cleanups into lp:ecryptfs.
>
> === modified file 'debian/changelog'
> --- debian/changelog 2013-06-08 01:37:48 +0000
> +++ debian/changelog 2013-06-27 20:57:33 +0000
> @@ -21,6 +21,11 @@
> - adjust man page text to avoid confusion about whether the
> interactive
> mount helper takes a capital 'N' for the answer to y/n questions
> (LP: #1130460)
> + * src/utils/ecryptfs_rewrap_passphrase.c:
> + - Handle errors when interactively reading the new wrapping passphrase
> + and the confirmation from stdin. Fixes a segfault (invalid memory
> read)
> + in ecryptfs-rewrap-passphrase if there was an error while reading
> either
> + of these passphrases.
>
> [ Dustin Kirkland ]
> * src/utils/ecryptfs-setup-swap: LP: #1172014
>
> === modified file 'src/utils/ecryptfs_rewrap_passphrase.c'
> --- src/utils/ecryptfs_rewrap_passphrase.c 2011-12-13 20:51:40 +0000
> +++ src/utils/ecryptfs_rewrap_passphrase.c 2013-06-27 20:57:33 +0000
> @@ -42,12 +42,13 @@
> char passphrase[ECRYPTFS_MAX_PASSWORD_LENGTH + 1];
> char *old_wrapping_passphrase;
> char *new_wrapping_passphrase;
> - char *new_wrapping_passphrase2;
> char salt[ECRYPTFS_SALT_SIZE];
> char salt_hex[ECRYPTFS_SALT_SIZE_HEX];
> int rc = 0;
>
> if (argc == 2) {
> + char *new_wrapping_passphrase2;
> +
> /* interactive mode */
> old_wrapping_passphrase =
> ecryptfs_get_passphrase("Old wrapping passphrase");
> @@ -55,10 +56,17 @@
> ecryptfs_get_passphrase("New wrapping passphrase");
> new_wrapping_passphrase2 =
> ecryptfs_get_passphrase("New wrapping passphrase
> (again)");
> - if (
> - strlen(new_wrapping_passphrase) !=
> strlen(new_wrapping_passphrase2) ||
> - strncmp(new_wrapping_passphrase,
> new_wrapping_passphrase2, strlen(new_wrapping_passphrase))!=0
> - ) {
> +
> + if (!new_wrapping_passphrase) {
> + fprintf(stderr, "Failed to read new wrapping
> passphrase\n");
> + rc = 1;
> + goto out;
> + } else if (!new_wrapping_passphrase2) {
> + fprintf(stderr, "Failed to read new wrapping
> passphrase confirmation\n");
> + rc = 1;
> + goto out;
> + } else if (strcmp(new_wrapping_passphrase,
> + ...

=== modified file 'debian/changelog'
--- debian/changelog	2013-06-08 01:37:48 +0000
+++ debian/changelog	2013-06-27 20:57:33 +0000
@@ -21,6 +21,11 @@
     - adjust man page text to avoid confusion about whether the interactive
       mount helper takes a capital 'N' for the answer to y/n questions
       (LP: #1130460)
+  * src/utils/ecryptfs_rewrap_passphrase.c:
+    - Handle errors when interactively reading the new wrapping passphrase
+      and the confirmation from stdin. Fixes a segfault (invalid memory read)
+      in ecryptfs-rewrap-passphrase if there was an error while reading either
+      of these passphrases.
 
   [ Dustin Kirkland ]
   * src/utils/ecryptfs-setup-swap: LP: #1172014

=== modified file 'src/utils/ecryptfs_rewrap_passphrase.c'
--- src/utils/ecryptfs_rewrap_passphrase.c	2011-12-13 20:51:40 +0000
+++ src/utils/ecryptfs_rewrap_passphrase.c	2013-06-27 20:57:33 +0000
@@ -42,12 +42,13 @@
 	char passphrase[ECRYPTFS_MAX_PASSWORD_LENGTH + 1];
 	char *old_wrapping_passphrase;
 	char *new_wrapping_passphrase;
-	char *new_wrapping_passphrase2;
 	char salt[ECRYPTFS_SALT_SIZE];
 	char salt_hex[ECRYPTFS_SALT_SIZE_HEX];
 	int rc = 0;
 
 	if (argc == 2) {
+		char *new_wrapping_passphrase2;
+
 		/* interactive mode */
 		old_wrapping_passphrase =
 			ecryptfs_get_passphrase("Old wrapping passphrase");
@@ -55,10 +56,17 @@
 			ecryptfs_get_passphrase("New wrapping passphrase");
 		new_wrapping_passphrase2 =
 			ecryptfs_get_passphrase("New wrapping passphrase (again)");
-		if (
-		    strlen(new_wrapping_passphrase) != strlen(new_wrapping_passphrase2) ||
-		    strncmp(new_wrapping_passphrase, new_wrapping_passphrase2, strlen(new_wrapping_passphrase))!=0
-		   ) {
+
+		if (!new_wrapping_passphrase) {
+			fprintf(stderr, "Failed to read new wrapping passphrase\n");
+			rc = 1;
+			goto out;
+		} else if (!new_wrapping_passphrase2) {
+			fprintf(stderr, "Failed to read new wrapping passphrase confirmation\n");
+			rc = 1;
+			goto out;
+		} else if (strcmp(new_wrapping_passphrase,
+				  new_wrapping_passphrase2)) {
 			fprintf(stderr, "New wrapping passphrases do not match\n");
 			rc = 1;
 			goto out;
@@ -76,7 +84,7 @@
 		usage();
 		goto out;
 	}
-	if (old_wrapping_passphrase==NULL || new_wrapping_passphrase==NULL ||
+	if (!old_wrapping_passphrase || !new_wrapping_passphrase ||
 	    strlen(old_wrapping_passphrase) > ECRYPTFS_MAX_PASSWORD_LENGTH ||
 	    strlen(new_wrapping_passphrase) > ECRYPTFS_MAX_PASSWORD_LENGTH) {
 		usage();