More defensive init; fix logic bug introduced earlier.

* lpass.c: Even more aggressive init, consume_entropy overwrites the
given buffer instead of relying on it being properly initialized.

1 files changed, 35 insertions, 33 deletions

diff --git a/lpass.c b/lpass.c
index b7a752d..4323a35 100644
--- a/lpass.c
+++ b/lpass.c
@@ -48,8 +48,7 @@ BIGNUM *
 calc_entropy(const char *site,
              const char *login,
              uint64_t counter,
-             const char *master_pass,
-             size_t passlen)
+             const char *master_pass)
 {
     char salt[MAX_BUF + 1];
     int saltlen = snprintf(salt, MAX_BUF, "%s%s%lx", site, login, counter);
@@ -58,7 +57,7 @@ calc_entropy(const char *site,
     }
 
     unsigned char key[ENTROPY_KEY_LENGTH];
-    int status = PKCS5_PBKDF2_HMAC(master_pass, passlen,
+    int status = PKCS5_PBKDF2_HMAC(master_pass, strlen(master_pass),
                                    (const unsigned char *) salt, saltlen,
                                    ENTROPY_ITERATIONS,
                                    EVP_sha256(),
@@ -89,9 +88,7 @@ consume_entropy(char *pass, BIGNUM *entropy, const char *charset, size_t maxlen)
         retval = 0;
         goto consume_entropy_cleanup;
     }
-    for (size_t passlen = strlen(pass);
-         passlen < maxlen;
-         passlen++) {
+    while (maxlen-- > 0) {
         BN_div(entropy, bn_remainder, entropy, bn_charsetlen, ctx);
 
         /*
@@ -105,9 +102,10 @@ consume_entropy(char *pass, BIGNUM *entropy, const char *charset, size_t maxlen)
             retval = 0;
             goto consume_entropy_cleanup;
         }
-        pass[passlen] = charset[remainder];
+        *pass++ = charset[remainder];
     }
 consume_entropy_cleanup:
+    *pass = '\0';
     BN_free(bn_remainder);
     BN_free(bn_charsetlen);
     BN_CTX_free(ctx);
@@ -161,6 +159,7 @@ insert_str_pseudo_randomly(char *pass, BIGNUM *entropy, const char *s)
         passlen++;
     }
 insert_str_pseudo_randomly_cleanup:
+    pass[passlen] = '\0';
     BN_free(bn_remainder);
     BN_free(bn_passlen);
     BN_CTX_free(ctx);
@@ -202,7 +201,6 @@ render_pass(BIGNUM *entropy, uint8_t allowed_charsets, char *out, size_t length)
 {
     char charset[MAX_BUF + 1];
     size_t num_charsets = build_charset(charset, allowed_charsets);
-    out[0] = '\0';
     if (consume_entropy(out, entropy, charset, length - num_charsets) == 0) {
         return 0;
     }
@@ -215,23 +213,19 @@ render_pass(BIGNUM *entropy, uint8_t allowed_charsets, char *out, size_t length)
     memset(str_to_add, 0, sizeof(str_to_add));
     size_t count = 0;
     if (charsets_has_set(allowed_charsets, CHARSET_LOWER)
-        && consume_entropy(&str_to_add[count], entropy, CHAR_SUBSET_LOWER, 1) == 0) {
-        count++;
+        && consume_entropy(&str_to_add[count++], entropy, CHAR_SUBSET_LOWER, 1) == 0) {
         return 0;
     }
     if (charsets_has_set(allowed_charsets, CHARSET_UPPER)
-        && consume_entropy(&str_to_add[count], entropy, CHAR_SUBSET_UPPER, 1) == 0) {
-        count++;
+        && consume_entropy(&str_to_add[count++], entropy, CHAR_SUBSET_UPPER, 1) == 0) {
         return 0;
     }
     if (charsets_has_set(allowed_charsets, CHARSET_DIGITS)
-        && consume_entropy(&str_to_add[count], entropy, CHAR_SUBSET_DIGITS, 1) == 0) {
-        count++;
+        && consume_entropy(&str_to_add[count++], entropy, CHAR_SUBSET_DIGITS, 1) == 0) {
         return 0;
     }
     if (charsets_has_set(allowed_charsets, CHARSET_SYMBOLS)
-        && consume_entropy(&str_to_add[count], entropy, CHAR_SUBSET_SYMBOLS, 1) == 0) {
-        count++;
+        && consume_entropy(&str_to_add[count++], entropy, CHAR_SUBSET_SYMBOLS, 1) == 0) {
         return 0;
     }
     return insert_str_pseudo_randomly(out, entropy, str_to_add);
@@ -253,13 +247,15 @@ usage()
     fputs("  SITE    The domain name or URL used in password generation.\n", stderr);
     fputs("  LOGIN   The username/login used in that domain or URL.\n", stderr);
     fputs("\nOptions:\n", stderr);
+    fputs("  -C COUNTER, --counter COUNTER\n", stderr);
+    fputs("                  password counter (default: 1)\n", stderr);
     fputs("  -L LENGTH, --length LENGTH\n", stderr);
-    fputs("                      password length (default: 16)\n", stderr);
-    fputs("  --no-lower          no lowercase letters in password\n", stderr);
-    fputs("  --no-upper          no uppercase letters in password\n", stderr);
-    fputs("  --no-digits         no digits in password\n", stderr);
-    fputs("  --no-symbols        no special symbols (" CHAR_SUBSET_SYMBOLS ")\n", stderr);
-    fputs("  -h, --help          output this message and exit\n", stderr);
+    fputs("                  password length (default: 16)\n", stderr);
+    fputs("  --no-lower      no lowercase letters in password\n", stderr);
+    fputs("  --no-upper      no uppercase letters in password\n", stderr);
+    fputs("  --no-digits     no digits in password\n", stderr);
+    fputs("  --no-symbols    no special symbols (" CHAR_SUBSET_SYMBOLS ")\n", stderr);
+    fputs("  -h, --help      output this message and exit\n", stderr);
     fputs("\nThe environment variable LESSPASS_MASTER_PASSWORD must be set with the master\n", stderr);
     fputs("password for producing the desired password.\n", stderr);
 }
@@ -267,8 +263,8 @@ usage()
 int
 main(int argc, char *argv[])
 {
-    int c;
-    size_t length = DEFAULT_LENGTH;
+    uint64_t pass_counter = 1;
+    size_t passlen = DEFAULT_LENGTH;
     uint8_t allowed_charsets = CHARSET_LOWER
         | CHARSET_UPPER
         | CHARSET_DIGITS
@@ -276,6 +272,7 @@ main(int argc, char *argv[])
     for (;;) {
         int option_index = 0;
         static struct option long_options[] = {
+            {"counter",    required_argument, 0, 'C'},
             {"help",       no_argument,       0, 'h'},
             {"length",     required_argument, 0, 'L'},
             {"no-lower",   no_argument,       0,  0 },
@@ -285,7 +282,7 @@ main(int argc, char *argv[])
             {0,            0,                 0,  0 }
         };
 
-        c = getopt_long(argc, argv, "hL:", long_options, &option_index);
+        int c = getopt_long(argc, argv, "hC:L:", long_options, &option_index);
         if (c == -1) {
             break;
         }
@@ -305,11 +302,18 @@ main(int argc, char *argv[])
         case 'h':
             usage();
             exit(EXIT_SUCCESS);
+        case 'C':
+            if (!optarg
+                || sscanf(optarg, "%lu", &pass_counter) == 0) {
+                usage();
+                exit(EXIT_FAILURE);
+            }
+            break;
         case 'L':
             if (!optarg
-                || sscanf(optarg, "%zu", &length) == 0
-                || length < 1
-                || length >= MAX_BUF) {
+                || sscanf(optarg, "%zu", &passlen) == 0
+                || passlen < 1
+                || passlen >= MAX_BUF) {
                 usage();
                 exit(EXIT_FAILURE);
             }
@@ -336,15 +340,13 @@ main(int argc, char *argv[])
 
     const char *site = argv[optind++];
     const char *login = argv[optind++];
-    size_t passlen = strlen(master_pass);
-    uint64_t counter = 1;
-    BIGNUM *entropy = calc_entropy(site, login, counter, master_pass, passlen);
+    BIGNUM *entropy = calc_entropy(site, login, pass_counter, master_pass);
     if (entropy == NULL) {
         err("Failed to calculate entropy");
     }
 
-    char generated_pass[length + 1];
-    if (render_pass(entropy, allowed_charsets, generated_pass, length) == 0) {
+    char generated_pass[passlen + 1];
+    if (render_pass(entropy, allowed_charsets, generated_pass, passlen) == 0) {
         BN_free(entropy);
         err("Failed to generate password");
     }