fix /costume core downer fix

[martysama0134 6943]

I'm not sure if it's already present, but people reported me the /costume could be exploited for crashing every server (even officials as far as I heard)

The bug is simple, the command prints the names of the costume's bonuses in chat. If they are missing in cmd_general.cpp fn_string, it returns null and causes a core crash, or if the format doesn't have a single-and-only-one format specifier %d.

In here the patch:

 

diff --git a/s3ll_server/README-SERVER.txt b/s3ll_server/README-SERVER.txt

index 94f67f8..7d1dd3f 100644

--- a/s3ll_server/README-SERVER.txt

+++ b/s3ll_server/README-SERVER.txt

@@ -336,6 +336,7 @@

+@fixme180: on cmd_general.cpp; /costume will cause game core crashes if the relative costume bonus ids aren't present inside fn_string or have no %d

 

 #@/Server (general)

 @fixme401: fixed the guild disband time issue

diff --git a/s3ll_server/Srcs/Server/game/src/cmd_general.cpp b/s3ll_server/Srcs/Server/game/src/cmd_general.cpp

index 739b7fc..740d80d 100644

--- a/s3ll_server/Srcs/Server/game/src/cmd_general.cpp

+++ b/s3ll_server/Srcs/Server/game/src/cmd_general.cpp

@@ -1871,8 +1871,8 @@ static const char* FN_point_string(int apply_number)

        case POINT_MALL_ATTBONUS:       return LC_TEXT("°ø°Ý·Â +%d%%");

        case POINT_MALL_DEFBONUS:       return LC_TEXT("¹æ¾î·Â +%d%%");

        case POINT_MALL_EXPBONUS:       return LC_TEXT("°æÇèÄ¡ %d%%");

-       case POINT_MALL_ITEMBONUS:      return LC_TEXT("¾ÆÀÌÅÛ µå·ÓÀ² %.1f¹è");

-       case POINT_MALL_GOLDBONUS:      return LC_TEXT("µ· µå·ÓÀ² %.1f¹è");

+       case POINT_MALL_ITEMBONUS:      return LC_TEXT("¾ÆÀÌÅÛ µå·ÓÀ² %d¹è"); // @fixme180 float to int

+       case POINT_MALL_GOLDBONUS:      return LC_TEXT("µ· µå·ÓÀ² %d¹è"); // @fixme180 float to int

        case POINT_MAX_HP_PCT:          return LC_TEXT("ÃÖ´ë »ý¸í·Â +%d%%");

        case POINT_MAX_SP_PCT:          return LC_TEXT("ÃÖ´ë Á¤½Å·Â +%d%%");

        case POINT_SKILL_DAMAGE_BONUS:  return LC_TEXT("½ºÅ³ µ¥¹ÌÁö %d%%");

@@ -1889,7 +1889,7 @@ static const char* FN_point_string(int apply_number)

 #ifdef ENABLE_WOLFMAN_CHARACTER

        case POINT_RESIST_WOLFMAN:  return LC_TEXT("¹«´ç°ø°Ý¿¡ %d%% ÀúÇ×");

 #endif

-       default:                    return NULL;

+       default:                    return "UNK_ID %d%%"; // @fixme180

    }

 }

 

You can try to refactor the return type as std::string to print the proper apply_number if you want, but it's not necessary.

A special thank to Tunga for being my guinea pig of the day

Edited May 4, 2020 by martysama0134

[VegaS™ 9934]

Thanks for the remark, but I never used this command in game as a player, this should be active just for debug as GM's, no sense for players.

About the fix, could be done directly from here too:

File: cmd_general.cpp

• costume

Search for:

				snprintf(buf, bufferSize, FN_point_string(attr.bType), attr.sValue);

Replace it with:

This is the hidden content, please

• Sign In
• or
• Sign Up

• hair

Search for:

	offset = snprintf(buf, bufsiz, FN_point_string(aff->bApplyOn), aff->lApplyValue);

Replace it with:

	const char * cPointString = FN_point_string(aff->bApplyOn);
	if (!*cPointString)
		return false;

	offset = snprintf(buf, bufsiz, cPointString, aff->lApplyValue);

 

There's no sense for showing to a player 'UNK... 23%' since he don't know what it's..

I think it's better just to ignore the type if doesn't exist and don't show it in the chat.

If you really want to do something like this, you can add a sys_log as an error to see the bonus missing and add it into the function.

Edited August 22, 2020 by VegaS™
added hair too

[TMP4 10476]

There are several sash system out, everyone should check if it's ok there too.

Mine looks ok by default:

    #ifdef __SASH_SYSTEM__
    if (pSash)
    {
        const char * itemName = pSash->GetName();
        ch->ChatPacket(CHAT_TYPE_INFO, "  SASH: %s", itemName);
        for (int i = 0; i < pSash->GetAttributeCount(); ++i)
        {
            const TPlayerItemAttribute& attr = pSash->GetAttribute(i);
            if (attr.bType > 0)
            {
                const char * pAttrName = FN_point_string(attr.bType);
                if (pAttrName == NULL)
                    continue;
                
                snprintf(buf, sizeof(buf), FN_point_string(attr.bType), attr.sValue);
                ch->ChatPacket(CHAT_TYPE_INFO, "     %s", buf);
            }
        }

        if (pSash->IsEquipped() && arg1[0] == 's')
            ch->UnequipItem(pSash);
    }
    #endif

Also it can be a positive thing to set the command's minimum requirements to implementor in cmd.cpp:

{ "costume",            do_costume,             0,    POS_DEAD,    GM_IMPLEMENTOR    },

Since player's don't need this command, it is only for debug as VegaS said.

 

Edit: Also check costume_weapon, but mine doesn't iterate through attrs so it's ok.

Edited May 4, 2020 by TMP4

[ilovegreendays 31]

Thx Martysama for that fix!

When u release teleport memory leak fix?

On some servers when teleport memory increase with no limit and dont reset. I saw a few of you talked about in 2016 post but no one gave a fix...

Edited May 4, 2020 by ilovegreendays

[SukH 855]

if (do_hair) look like this  

 

ACMD(do_hair)
{
    char buf[256];

    if (false == FN_hair_affect_string(ch, buf, sizeof(buf)))
        return;

    ch->ChatPacket(CHAT_TYPE_INFO, buf);
}

so i change to this or what ?

ACMD(do_hair)
{
    char buf[256];

    const char * cPointString = FN_point_string(attr.bType);
                if (!*cPointString)
                    continue;

        snprintf(buf, bufferSize, cPointString, attr.sValue);
        return;

    ch->ChatPacket(CHAT_TYPE_INFO, buf);
}

 

[VegaS™ 9934]

@Arkane2 Check my first reply, i added it too.

[SukH 855]

@VegaS™ i just removed those command because it`s not usefull anymore i guess xd better than fix it

Edited May 5, 2020 by Arkane2