Bug 2527: PASSWORD command handling of bad passwords.

res/translations/messages.de.po

@@ -2429,6 +2429,9 @@ msgstr "\"$unit($unit) konnte nur $int($ships) von $int($maxships) Schiffen verz
 msgid "error283"
 msgstr "\"$unit($unit) in $region($region): '$order($command)' - Das Passwort darf nur Buchstaben und Ziffern enthalten.\""
 
+msgid "error321"
+msgstr "\"$unit($unit) in $region($region): '$order($command)' - Das gewählte Passwort war zu lang.\""
+
 msgid "rust_effect"
 msgstr "\"$unit($mage) legt einen Rosthauch auf $unit($target). $int($amount) Waffen wurden vom Rost zerfressen.\""
 

res/translations/messages.en.po

@@ -2426,6 +2426,9 @@ msgstr "\"$unit($unit) in $region($region): '$order($command)' - No luxury items
 msgid "stormwinds_reduced"
 msgstr "\"$unit($unit) could only enchant $int($ships) of $int($maxships) ships.\""
 
+msgid "error321"
+msgstr "\"$unit($unit) in $region($region): '$order($command)' - The chosen password was too long.\""
+
 msgid "error283"
 msgstr "\"$unit($unit) in $region($region): '$order($command)' - Your password may only contain alphanumeric symbols.\""
 

src/laws.c

@@ -2088,34 +2088,33 @@ int email_cmd(unit * u, struct order *ord)
 
 int password_cmd(unit * u, struct order *ord)
 {
-    char pwbuf[32];
+    char pwbuf[PASSWORD_MAXSIZE + 1];
     const char *s;
-    bool pwok = true;
 
     init_order_depr(ord);
+    pwbuf[PASSWORD_MAXSIZE] = '\n';
     s = gettoken(pwbuf, sizeof(pwbuf));
-
-    if (!s || !*s) {
-        int i;
-        for (i = 0; i < 6; i++)
-            pwbuf[i] = (char)(97 + rng_int() % 26);
-        pwbuf[6] = 0;
+    if (pwbuf[PASSWORD_MAXSIZE] == '\0') {
+        cmistake(u, ord, 321, MSG_EVENT);
+        pwbuf[PASSWORD_MAXSIZE - 1] = '\0';
     }
-    else {
-        char *c;
-        for (c = pwbuf; *c && pwok; ++c) {
-            if (!isalnum(*(unsigned char *)c)) {
-                pwok = false;
+
+    if (s && *s) {
+        unsigned char *c = (unsigned char *)pwbuf;
+        int i, r = 0;
+
+        for (i = 0; c[i] && i != PASSWORD_MAXSIZE; ++i) {
+            if (!isalnum(c[i])) {
+                c[i] = 'X';
+                ++r;
             }
         }
-    }
-    if (!pwok) {
-        cmistake(u, ord, 283, MSG_EVENT);
-        str_strlcpy(pwbuf, itoa36(rng_int()), sizeof(pwbuf));
+        if (r != 0) {
+            cmistake(u, ord, 283, MSG_EVENT);
+        }
     }
     faction_setpassword(u->faction, password_hash(pwbuf, PASSWORD_DEFAULT));
-    ADDMSG(&u->faction->msgs, msg_message("changepasswd",
-        "value", pwbuf));
+    ADDMSG(&u->faction->msgs, msg_message("changepasswd", "value", pwbuf));
     u->faction->flags |= FFL_PWMSG;
     return 0;
 }

src/laws.test.c

@@ -48,6 +48,37 @@ static void test_new_building_can_be_renamed(CuTest * tc)
     test_teardown();
 }
 
+static void test_password_cmd(CuTest * tc)
+{
+    unit *u;
+    faction * f;
+    test_setup();
+    u = test_create_unit(f = test_create_faction(NULL), test_create_plain(0, 0));
+
+    u->thisorder = create_order(K_PASSWORD, f->locale, "abcdefgh");
+    password_cmd(u, u->thisorder);
+    CuAssertPtrNotNull(tc, faction_getpassword(f));
+    CuAssertTrue(tc, checkpasswd(f, "abcdefgh"));
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "changepasswd"));
+    free_order(u->thisorder);
+
+    u->thisorder = create_order(K_PASSWORD, f->locale, "abc*de*");
+    password_cmd(u, u->thisorder);
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error283"));
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "changepasswd"));
+    CuAssertTrue(tc, !checkpasswd(f, "abc*de*"));
+    CuAssertTrue(tc, checkpasswd(f, "abcXdeX"));
+    free_order(u->thisorder);
+
+    u->thisorder = create_order(K_PASSWORD, f->locale, "1234567890123456789012345678901234567890");
+    password_cmd(u, u->thisorder);
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "error321"));
+    CuAssertPtrNotNull(tc, test_find_messagetype(f->msgs, "changepasswd"));
+    CuAssertTrue(tc, checkpasswd(f, "1234567890123456789012345678901"));
+
+    test_teardown();
+}
+
 static void test_rename_building(CuTest * tc)
 {
     region *r;
@@ -1831,6 +1862,7 @@ CuSuite *get_laws_suite(void)
     SUITE_ADD_TEST(suite, test_long_order_buy_cast);
     SUITE_ADD_TEST(suite, test_long_order_hungry);
     SUITE_ADD_TEST(suite, test_new_building_can_be_renamed);
+    SUITE_ADD_TEST(suite, test_password_cmd);
     SUITE_ADD_TEST(suite, test_rename_building);
     SUITE_ADD_TEST(suite, test_rename_building_twice);
     SUITE_ADD_TEST(suite, test_fishing_feeds_2_people);

src/orderfile.c

@@ -14,6 +14,7 @@
 #include "util/filereader.h"
 #include "util/param.h"
 #include "util/parser.h"
+#include "util/password.h"
 #include "util/order_parser.h"
 
 #include <assert.h>
@@ -127,7 +128,7 @@ static faction *factionorders(void)
     faction *f = findfaction(fid);
 
     if (f != NULL && (f->flags & FFL_NPC) == 0) {
-        char token[128];
+        char token[PASSWORD_MAXSIZE];
         const char *pass = gettoken(token, sizeof(token));
 
         if (!checkpasswd(f, (const char *)pass)) {

src/util/password.h

@@ -6,6 +6,7 @@ typedef enum cryptalgo_t {
     PASSWORD_BCRYPT
 } cryptalgo_t;
 #define PASSWORD_DEFAULT PASSWORD_BCRYPT
+#define PASSWORD_MAXSIZE 32
 
 extern int bcrypt_workfactor;