Array overrun and incorrect string macros used in do_setclass
< Newer Topic
:: Older Topic >
Pages:<< prev 1 next >>
#1 Jan 7, 2025 3:20 am
Last edited Jan 7, 2025 3:39 am by Samson
Black Hand
GroupAdministrators
Posts3,713
JoinedJan 1, 2002
GitHub commit: https://github.com/Arthmoor/SmaugFUSS/commit/8981a76359dc31d05a95617af1a2bb0081c4dbf5
This is a pretty major one. At some point in the past, SmaugFUSS added the ability to change class titles online. In doing so though, the wrong set of string macros were used AND the array references were out of bounds because it was apparently assumed that SEX_MALE and SEX_FEMALE were the only two gender variables available. However, Smaug itself has always had a SEX_NEUTRAL one as well which occupied the 0 slot on the enum, which made SEX_MALE occupy 1, and SEX_FEMALE occupy 2. The title_table array only has TWO slots though. One for males, which it assumes is the 0 slot, and one for females which it assumes is the 1 slot.
Defined as such:
Anyone attempting to set male titles this way probably wondered why they kept coming up for females instead, and obviously trying to set female ones would either crash the game or corrupt data in memory.
The following fix has been committed to GitHub and will be in the 1.9.6 release:
In do_setclass, find:
Change it to read as follows:
And while we're at it, appreciate the irony of Shaddai's attempt to fix female titles not ever being settable because the line below his comment was missing at some point in the past
This is a pretty major one. At some point in the past, SmaugFUSS added the ability to change class titles online. In doing so though, the wrong set of string macros were used AND the array references were out of bounds because it was apparently assumed that SEX_MALE and SEX_FEMALE were the only two gender variables available. However, Smaug itself has always had a SEX_NEUTRAL one as well which occupied the 0 slot on the enum, which made SEX_MALE occupy 1, and SEX_FEMALE occupy 2. The title_table array only has TWO slots though. One for males, which it assumes is the 0 slot, and one for females which it assumes is the 1 slot.
Defined as such:
const char *title_table[MAX_CLASS][MAX_LEVEL + 1][2];
Anyone attempting to set male titles this way probably wondered why they kept coming up for females instead, and obviously trying to set female ones would either crash the game or corrupt data in memory.
The following fix has been committed to GitHub and will be in the 1.9.6 release:
In do_setclass, find:
if( !str_cmp( arg2, "mtitle" ) ) { char arg3[MAX_INPUT_LENGTH]; int x; argument = one_argument( argument, arg3 ); if( arg3[0] == '\0' || argument[0] == '\0' ) { send_to_char( "Syntax: setclass <class> mtitle <level> <title>\r\n", ch ); return; } if( ( x = atoi( arg3 ) ) < 0 || x > MAX_LEVEL ) { send_to_char( "Invalid level.\r\n", ch ); return; } STRFREE( title_table[cl][x][SEX_MALE] ); title_table[cl][x][SEX_MALE] = STRALLOC( argument ); send_to_char( "Done.\r\n", ch ); write_class_file( cl ); return; } if( !str_cmp( arg2, "ftitle" ) ) { char arg3[MAX_INPUT_LENGTH], arg4[MAX_INPUT_LENGTH]; int x; argument = one_argument( argument, arg3 ); argument = one_argument( argument, arg4 ); if( arg3[0] == '\0' || argument[0] == '\0' ) { send_to_char( "Syntax: setclass <class> ftitle <level> <title>\r\n", ch ); return; } if( ( x = atoi( arg4 ) ) < 0 || x > MAX_LEVEL ) { send_to_char( "Invalid level.\r\n", ch ); return; } STRFREE( title_table[cl][x][SEX_FEMALE] ); /* * Bug fix below -Shaddai */ title_table[cl][x][SEX_FEMALE] = STRALLOC( argument ); send_to_char( "Done\r\n", ch ); write_class_file( cl ); return; }
Change it to read as follows:
if( !str_cmp( arg2, "mtitle" ) ) { char arg3[MAX_INPUT_LENGTH]; int x; argument = one_argument( argument, arg3 ); if( arg3[0] == '\0' || argument[0] == '\0' ) { send_to_char( "Syntax: setclass <class> mtitle <level> <title>\r\n", ch ); return; } if( ( x = atoi( arg3 ) ) < 0 || x > MAX_LEVEL ) { send_to_char( "Invalid level.\r\n", ch ); return; } DISPOSE( title_table[cl][x][0] ); title_table[cl][x][0] = str_dup( argument ); send_to_char( "Done.\r\n", ch ); write_class_file( cl ); return; } if( !str_cmp( arg2, "ftitle" ) ) { char arg3[MAX_INPUT_LENGTH], arg4[MAX_INPUT_LENGTH]; int x; argument = one_argument( argument, arg3 ); argument = one_argument( argument, arg4 ); if( arg3[0] == '\0' || argument[0] == '\0' ) { send_to_char( "Syntax: setclass <class> ftitle <level> <title>\r\n", ch ); return; } if( ( x = atoi( arg4 ) ) < 0 || x > MAX_LEVEL ) { send_to_char( "Invalid level.\r\n", ch ); return; } DISPOSE( title_table[cl][x][1] ); /* * Bug fix below -Shaddai */ title_table[cl][x][1] = str_dup( argument ); send_to_char( "Done\r\n", ch ); write_class_file( cl ); return; }
And while we're at it, appreciate the irony of Shaddai's attempt to fix female titles not ever being settable because the line below his comment was missing at some point in the past
Pages:<< prev 1 next >>