diff --git a/Server/GameServer/Game/Struct/CalculateStat.cpp b/Server/GameServer/Game/Struct/CalculateStat.cpp index 3d4d7ea..b324993 100644 --- a/Server/GameServer/Game/Struct/CalculateStat.cpp +++ b/Server/GameServer/Game/Struct/CalculateStat.cpp @@ -6138,19 +6138,38 @@ void StructCreature::checkAdditionalItemEffect() // Fraun Sky Accessories 7/12/2025 + // Fix: the bonus is stored as (bitset, base, per-level) triples - the same layout the + // engine uses for EF_PARAMETER_INC (see _applyEffectForState: incParameter(fValue[k], + // fValue[k+1] + fValue[k+2]*level, ...)) and the client tooltip + // (ReplaceAdditionalItemEffectOptionTooltip walks value[n]/[n+1]/[n+2], stride 3). + // The old loop walked the triples but always read fValue[1]/[2] of triple 0, passing + // the *amount* as the stat selector bitset and the *per-level* term (normally 0) as the + // value -> worn sky accessories granted nothing even though the tooltip advertised a + // bonus. Apply each non-empty triple the same way the engine does instead. if (pEquipment->GetItemAdditionalEffect() > 0) { const std::vector* Effect = GameContent::GetEffectInfoVector(pEquipment->GetItemAdditionalEffect()); if (Effect->size() > 0) { - for (int eff = 0; eff < 20; ) + EffectInfo * pSkyEffect = Effect->at(0); + + // 6 triples fit in fValue[NUMBER_OF_VALUE(=20)]; stop at eff+2 < NUMBER_OF_VALUE + // so fValue[eff+1] / fValue[eff+2] stay in bounds. + for (int eff = 0; eff + 2 < EffectInfo::NUMBER_OF_VALUE; eff += 3) { - if (Effect->at(0)->fValue[eff] != 0) - { - incParameter(Effect->at(0)->fValue[1], Effect->at(0)->fValue[2], true); - } - eff += 3; + if (pSkyEffect->fValue[eff] == 0) + continue; + + const c_fixed10 fAmount = pSkyEffect->fValue[eff + 1] + pSkyEffect->fValue[eff + 2] * pSkyEffect->nEffectLevel; + + // fValue[eff] is the stat/attribute selector bitset. Apply it both as a base + // stat (STR/VIT/... under bStat=true) and as an attribute (attack/defence/... + // under bStat=false): the two branches of incParameter() test disjoint flag + // sets, so whichever the effect row encodes is applied exactly once and the + // result matches what the client tooltip shows. + incParameter(pSkyEffect->fValue[eff], fAmount, true); + incParameter(pSkyEffect->fValue[eff], fAmount, false); } } } diff --git a/Server/GameServer/Game/Struct/StructPlayer.cpp b/Server/GameServer/Game/Struct/StructPlayer.cpp index 94f6c95..0bd1847 100644 --- a/Server/GameServer/Game/Struct/StructPlayer.cpp +++ b/Server/GameServer/Game/Struct/StructPlayer.cpp @@ -5573,6 +5573,68 @@ void StructPlayer::SetDialogTitle( const char *szString, int type ) m_strSpecialDialogMenu.clear(); } +// --- Fraun security fix: NPC dialog trigger validation ------------------------------- +// The client echoes dialog triggers back to the server, which then runs them through +// LUA()->RunString() (see onDialog in GameMessage.cpp). Validation used to be a substring +// test (strstr), so a crafted / man-in-the-middled TS_CS_DIALOG packet could embed a +// legitimate trigger substring and append arbitrary Lua, e.g. +// on_channel_set(1) os.execute("...") +// Because the Lua VM is opened with luaL_openlibs (os/io included), that is full remote +// code execution on the server host. The validators below now constrain the trigger to +// exactly the shapes the real client can produce. +// +// A "special" dialog menu is a parameterised trigger: the server hands out a prefix +// (e.g. "on_channel_set", "warp_to_secret_dungeon") and the client completes it with a +// numeric argument list -> "prefix()" (client builds it as "%s(%s)" / "%s()" in +// SUIInputNumberWnd / SGameInterface). This helper accepts ONLY that shape: the trigger +// must start with the prefix and the remainder may contain nothing but a single +// parenthesised list of digits / signs / dots / commas / whitespace. That lets the client +// pick a number while making it impossible to chain extra Lua statements. +static bool IsSafeSpecialDialogTrigger( const char * szTrigger, const char * szPrefix ) +{ + if( !szTrigger || !szPrefix || !*szPrefix ) + return false; + + const size_t nPrefixLen = strlen( szPrefix ); + + // Must START with the offered prefix (not merely contain it). + if( strncmp( szTrigger, szPrefix, nPrefixLen ) != 0 ) + return false; + + const char * p = szTrigger + nPrefixLen; + + while( *p == ' ' || *p == '\t' ) ++p; // optional whitespace before '(' + + if( *p == '\0' ) + return true; // bare prefix, e.g. "exit_indun" + + if( *p != '(' ) + return false; // anything other than an arg list -> reject + ++p; + + // Argument list: numbers only. No quotes, letters, ';', '=', '[', a second '(' etc., + // so no further Lua statement can be smuggled in. + while( *p && *p != ')' ) + { + const char c = *p; + const bool bAllowed = + ( c >= '0' && c <= '9' ) || + c == ',' || c == '.' || c == '-' || c == '+' || c == ' ' || c == '\t'; + if( !bAllowed ) + return false; + ++p; + } + + if( *p != ')' ) + return false; // unterminated argument list + ++p; + + while( *p == ' ' || *p == '\t' ) ++p; // optional trailing whitespace + + return *p == '\0'; // nothing may follow the ')' +} +// ------------------------------------------------------------------------------------- + void StructPlayer::SetFixedDialogTrigger( const char *szString ) { m_strFixedDialogTrigger = szString; @@ -5580,7 +5642,11 @@ void StructPlayer::SetFixedDialogTrigger( const char *szString ) bool StructPlayer::IsFixedDialogTrigger( const char * szTrigger ) { - if( m_strFixedDialogTrigger.size() && strstr( szTrigger, m_strFixedDialogTrigger.c_str() ) ) + // Fraun security fix: the fixed trigger is a complete, server-generated string that the + // client echoes back verbatim (e.g. "recall_feather( x, y, layer )" -> SUINotifyWnd sends + // it back with "%s"), so it must match EXACTLY. strstr() let a crafted packet keep the + // trigger as a prefix and append Lua after it. + if( m_strFixedDialogTrigger.size() && !strcmp( szTrigger, m_strFixedDialogTrigger.c_str() ) ) return true; return false; @@ -5614,7 +5680,8 @@ void StructPlayer::SetSpecialDialogMenu( const char * szString ) bool StructPlayer::IsSpecialDialogMenu( const char * szString ) { - return( m_strSpecialDialogMenu.size() && strstr( szString, m_strSpecialDialogMenu.c_str() ) ); + // Fraun security fix: was strstr() (substring) -> now require "()". + return( m_strSpecialDialogMenu.size() && IsSafeSpecialDialogTrigger( szString, m_strSpecialDialogMenu.c_str() ) ); } void StructPlayer::ClearSpecialDialogMenu() @@ -5769,7 +5836,8 @@ bool StructPlayer::IsValidTrigger( const char *szTrigger ) if( !strcmp( strTriggerList[idx].c_str(), szTrigger ) ) return true; } - if( m_strSpecialDialogMenu.size() && strstr( szTrigger, m_strSpecialDialogMenu.c_str() ) ) + // Fraun security fix: was strstr() (substring) -> now require "()". + if( m_strSpecialDialogMenu.size() && IsSafeSpecialDialogTrigger( szTrigger, m_strSpecialDialogMenu.c_str() ) ) return true; return false;