Welcome to the Builder Academy

Question MUD Crash on logon - crypt() strncmp()

More
30 Jun 2015 05:59 #5355 by krell
MUD Crash on logon - crypt() strncmp() was created by krell
Recently I've been having trouble logging into my locally running version of tba. If I entered they password incorrectly the MUD would just crash. I'm running OpenBSD -current and so I suspected that there was a change in the supporting libraries on my os. So I grabbed the most recent version of tba, compiled and ran it...the MUD still crashed.

I fired up gdb after recompiling the MUD without optimizations and looked at the fresh crash file. Apparently strncmp() is crashing when it calls crypt() through the tbaMUD macros. It turns out that crypt() returns a NULL pointer when the entered password doesn't match the stored password, but strncmp() doesn't define any behavior for receiving a NULL pointer. Evidently the OBSD devs felt that the function should crash on receiving a unallocated pointer such as a NULL.

I've made the following diffs that appear to work. Please view, comment and test.

src/utils.c
Code:
--- ../tbamud/src/utils.c.orig Thu Jun 25 02:55:37 2015 +++ ../tbamud/src/utils.c Tue Jun 30 02:59:51 2015 @@ -1533,3 +1533,21 @@ parse_tab(buf); return(buf); } + +/* + * NULL pointers are undefined in strncmp() for many os's and + * architectures. This function returns an empty string rather than + * an NULL pointer since NULL pointers can crash some versions of + * strncmp(). + * + * Pre: kstr must be a valid char pointer to user input. + * pstr must be a valid char pointer to password database. + */ + +const char * wrap_crypt(const char *kstr, const char *pstr) +{ + if (crypt(kstr,pstr)) + return(pstr); + else + return(kstr); +}

src/utils.h
Code:
--- ../tbamud/src/utils.h.orig Thu Jun 25 02:54:36 2015 +++ ../tbamud/src/utils.h Thu Jun 25 05:55:38 2015 @@ -70,6 +70,8 @@ int get_class_by_name(char *classname); char * convert_from_tabs(char * string); int count_non_protocol_chars(char * str); +/* crypt wrapper utility */ +const char * wrap_crypt(const char *, const char *); /* Public functions made available form weather.c */ void weather_and_time(int mode); @@ -929,7 +931,7 @@ #define CRYPT(a,b) (a) #else /** When crypt is defined. Player passwords stored encrypted. */ -#define CRYPT(a,b) ((char *) crypt((a),(b))) +#define CRYPT(a,b) (wrap_crypt((a),(b))) #endif /* Config macros */

www.openbsd.org/cgi-bin/man.cgi/OpenBSD-....3?query=crypt&sec=3
www.openbsd.org/cgi-bin/man.cgi?query=st...rch=i386&format=html

Please Log in or Create an account to join the conversation.

More
30 Jun 2015 06:42 - 30 Jun 2015 13:57 #5356 by krell
Replied by krell on topic MUD Crash on logon - crypt() strncmp()
In the diff for src/utils.c I obviously mean that a valid pointer to the user entered string is returned rather than a NULL pointer. I'll have to fix my official diff to reflect that.

***
updated src/utils.c diff
Code:
--- ../tbamud/src/utils.c.orig Thu Jun 25 02:55:37 2015 +++ ../tbamud/src/utils.c Tue Jun 30 07:00:17 2015 @@ -1533,3 +1533,21 @@ parse_tab(buf); return(buf); } + +/* + * NULL pointers are undefined in strncmp() for many os's and + * architectures. This function returns a valid pointer to the + * user entered string rather than an NULL pointer since NULL + * pointers can crash some versions of strncmp(). + * + * Pre: kstr must be a valid char pointer to user input. + * pstr must be a valid char pointer to password database. + */ + +const char * wrap_crypt(const char *kstr, const char *pstr) +{ + if (crypt(kstr,pstr)) + return(pstr); + else + return(kstr); +}
Last edit: 30 Jun 2015 13:57 by krell. Reason: Add slightly modified diff

Please Log in or Create an account to join the conversation.

More
30 Jun 2015 16:28 #5357 by Kyle
Replied by Kyle on topic MUD Crash on logon - crypt() strncmp()
Nice work!

Please Log in or Create an account to join the conversation.

More
30 Jun 2015 16:46 #5358 by Kyle
Replied by Kyle on topic MUD Crash on logon - crypt() strncmp()
alternatively, to avoid sending a null character pointer to strncmp, something like
Code:
if (CRYPT(arg, GET_PASSWD(d->character)) && strncmp(CRYPT(arg, GET_PASSWD(d->character)), GET_PASSWD(d->character), MAX_PWD_LENGTH))
could be used. then strncmp will only be called if its first parameter is non-null?

of course then an appropriate else if() would have to be added to ensure that passwords cannot be bypassed by simply not entering one.

your wrapper solution avoids further convoluting the already horrific nanny() function :P

Please Log in or Create an account to join the conversation.

More
30 Jun 2015 17:56 #5359 by krell
Replied by krell on topic MUD Crash on logon - crypt() strncmp()
Thanks, I was hoping to make my solution as simple as possible and still work without unintended side effects. It was because nanny() was already so complicated I didn't want to "out clever" myself.

Please Log in or Create an account to join the conversation.

More
05 Jul 2015 20:34 #5408 by krell
Replied by krell on topic MUD Crash on logon - crypt() strncmp()
Been playing around with the modifications I've made to my local copy of they mud and learned a couple of things. First off, my passwords aren't being encrypted and secondly, there is a function in asciipasswd.c that doesn't like my redefined macro. Here are my most recent diffs, the one for utils.h has changed and I have one for asciipasswd.c

src/utils.h
Code:
--- ../tbamud/src/utils.h Mon Jun 22 00:20:14 2015 +++ ../tbamud/src/utils.h Sun Jul 5 16:27:40 2015 @@ -70,6 +70,8 @@ int get_class_by_name(char *classname); char * convert_from_tabs(char * string); int count_non_protocol_chars(char * str); +/* crypt wrapper utility */ +const char * wrap_crypt(const char *, const char *); /* Public functions made available form weather.c */ void weather_and_time(int mode); @@ -927,9 +929,11 @@ #if defined(NOCRYPT) || !defined(CIRCLE_CRYPT) /** When crypt is not defined. (NOTE: Player passwords will be plain text.) */ #define CRYPT(a,b) (a) +#define ACRYPT(a,b) (a) #else /** When crypt is defined. Player passwords stored encrypted. */ -#define CRYPT(a,b) ((char *) crypt((a),(b))) +#define CRYPT(a,b) (wrap_crypt((a),(b))) +#define ACRYPT(a,b) ((char *) crypt((a),(b))) #endif /* Config macros */

src/util/asciipasswd.c
Code:
--- ../tbamud/src/util/asciipasswd.c Sun Jun 9 12:57:48 2013 +++ ../tbamud/src/util/asciipasswd.c Sun Jul 5 16:24:31 2015 @@ -22,7 +22,7 @@ if (argc != 3) fprintf(stderr, "Usage: %s name password\n", argv[0]); else - printf("Name: %s\nPass: %s\n", CAP(argv[1]), CRYPT(argv[2], CAP(argv[1]))); + printf("Name: %s\nPass: %s\n", CAP(argv[1]), ACRYPT(argv[2], CAP(argv[1]))); return (0); }

If I could get people to review, test and comment I'd appreciate it. Thanks.

Please Log in or Create an account to join the conversation.

Time to create page: 0.189 seconds

tbaMUD © 2024