From d452bcda8fce9ce294a92bb61b94192f585a3460 Mon Sep 17 00:00:00 2001 From: KebsCS Date: Tue, 27 Aug 2024 22:47:24 +0200 Subject: [PATCH] Add validation for chat and console command arguments --- src/engine/console.h | 2 +- src/engine/shared/config.cpp | 25 +++++++----- src/engine/shared/console.cpp | 70 ++++++++++++++++++++++++--------- src/engine/shared/console.h | 13 +++++- src/rust-bridge/cpp/console.cpp | 4 +- 5 files changed, 81 insertions(+), 33 deletions(-) diff --git a/src/engine/console.h b/src/engine/console.h index 0c04678eb..ce79012d8 100644 --- a/src/engine/console.h +++ b/src/engine/console.h @@ -53,7 +53,7 @@ public: virtual int GetInteger(unsigned Index) const = 0; virtual float GetFloat(unsigned Index) const = 0; virtual const char *GetString(unsigned Index) const = 0; - virtual ColorHSLA GetColor(unsigned Index, bool Light) const = 0; + virtual std::optional GetColor(unsigned Index, bool Light) const = 0; virtual void RemoveArgument(unsigned Index) = 0; diff --git a/src/engine/shared/config.cpp b/src/engine/shared/config.cpp index 1d78696e3..6c71c4b11 100644 --- a/src/engine/shared/config.cpp +++ b/src/engine/shared/config.cpp @@ -111,22 +111,29 @@ void SIntConfigVariable::ResetToOld() void SColorConfigVariable::CommandCallback(IConsole::IResult *pResult, void *pUserData) { SColorConfigVariable *pData = static_cast(pUserData); - + char aBuf[IConsole::CMDLINE_LENGTH + 64]; if(pResult->NumArguments()) { if(pData->CheckReadOnly()) return; - const ColorHSLA Color = pResult->GetColor(0, pData->m_Light); - const unsigned Value = Color.Pack(pData->m_Light ? 0.5f : 0.0f, pData->m_Alpha); + const auto Color = pResult->GetColor(0, pData->m_Light); + if(Color) + { + const unsigned Value = Color->Pack(pData->m_Light ? 0.5f : 0.0f, pData->m_Alpha); - *pData->m_pVariable = Value; - if(pResult->m_ClientId != IConsole::CLIENT_ID_GAME) - pData->m_OldValue = Value; + *pData->m_pVariable = Value; + if(pResult->m_ClientId != IConsole::CLIENT_ID_GAME) + pData->m_OldValue = Value; + } + else + { + str_format(aBuf, sizeof(aBuf), "%s is not a valid color.", pResult->GetString(0)); + pData->m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "config", aBuf); + } } else { - char aBuf[256]; str_format(aBuf, sizeof(aBuf), "Value: %u", *pData->m_pVariable); pData->m_pConsole->Print(IConsole::OUTPUT_LEVEL_STANDARD, "config", aBuf); @@ -493,8 +500,8 @@ void CConfigManager::Con_Toggle(IConsole::IResult *pResult, void *pUserData) { SColorConfigVariable *pColorVariable = static_cast(pVariable); const float Darkest = pColorVariable->m_Light ? 0.5f : 0.0f; - const ColorHSLA Value = *pColorVariable->m_pVariable == pResult->GetColor(1, pColorVariable->m_Light).Pack(Darkest, pColorVariable->m_Alpha) ? pResult->GetColor(2, pColorVariable->m_Light) : pResult->GetColor(1, pColorVariable->m_Light); - pColorVariable->SetValue(Value.Pack(Darkest, pColorVariable->m_Alpha)); + const std::optional Value = *pColorVariable->m_pVariable == pResult->GetColor(1, pColorVariable->m_Light).value_or(ColorHSLA(0, 0, 0)).Pack(Darkest, pColorVariable->m_Alpha) ? pResult->GetColor(2, pColorVariable->m_Light) : pResult->GetColor(1, pColorVariable->m_Light); + pColorVariable->SetValue(Value.value_or(ColorHSLA(0, 0, 0)).Pack(Darkest, pColorVariable->m_Alpha)); } else if(pVariable->m_Type == SConfigVariable::VAR_STRING) { diff --git a/src/engine/shared/console.cpp b/src/engine/shared/console.cpp index 29610d796..7c8a84ad6 100644 --- a/src/engine/shared/console.cpp +++ b/src/engine/shared/console.cpp @@ -40,22 +40,29 @@ float CConsole::CResult::GetFloat(unsigned Index) const return str_tofloat(m_apArgs[Index]); } -ColorHSLA CConsole::CResult::GetColor(unsigned Index, bool Light) const +std::optional CConsole::CResult::GetColor(unsigned Index, bool Light) const { if(Index >= m_NumArgs) - return ColorHSLA(0, 0, 0); + return std::nullopt; const char *pStr = m_apArgs[Index]; if(str_isallnum(pStr) || ((pStr[0] == '-' || pStr[0] == '+') && str_isallnum(pStr + 1))) // Teeworlds Color (Packed HSL) { - const ColorHSLA Hsla = ColorHSLA(str_toulong_base(pStr, 10), true); + unsigned long Value = str_toulong_base(pStr, 10); + if(Value == std::numeric_limits::max()) + return std::nullopt; + const ColorHSLA Hsla = ColorHSLA(Value, true); if(Light) return Hsla.UnclampLighting(); return Hsla; } else if(*pStr == '$') // Hex RGB/RGBA { - return color_cast(color_parse(pStr + 1).value_or(ColorRGBA(0.0f, 0.0f, 0.0f, 1.0f))); + auto ParsedColor = color_parse(pStr + 1); + if(ParsedColor) + return color_cast(ParsedColor.value()); + else + return std::nullopt; } else if(!str_comp_nocase(pStr, "red")) return ColorHSLA(0.0f / 6.0f, 1, .5f); @@ -76,7 +83,7 @@ ColorHSLA CConsole::CResult::GetColor(unsigned Index, bool Light) const else if(!str_comp_nocase(pStr, "black")) return ColorHSLA(0, 0, 0); - return ColorHSLA(0, 0, 0); + return std::nullopt; } const IConsole::CCommandInfo *CConsole::CCommand::NextCommandInfo(int AccessLevel, int FlagMask) const @@ -129,12 +136,12 @@ int CConsole::ParseStart(CResult *pResult, const char *pString, int Length) return 0; } -int CConsole::ParseArgs(CResult *pResult, const char *pFormat) +int CConsole::ParseArgs(CResult *pResult, const char *pFormat, FCommandCallback pfnCallback) { char Command = *pFormat; char *pStr; int Optional = 0; - int Error = 0; + int Error = PARSEARGS_OK; pResult->ResetVictim(); @@ -155,7 +162,7 @@ int CConsole::ParseArgs(CResult *pResult, const char *pFormat) { if(!Optional) { - Error = 1; + Error = PARSEARGS_MISSING_VALUE; break; } @@ -191,7 +198,7 @@ int CConsole::ParseArgs(CResult *pResult, const char *pFormat) pStr++; // skip due to escape } else if(pStr[0] == 0) - return 1; // return error + return PARSEARGS_MISSING_VALUE; // return error *pDst = *pStr; pDst++; @@ -215,13 +222,7 @@ int CConsole::ParseArgs(CResult *pResult, const char *pFormat) if(Command == 'r') // rest of the string break; - else if(Command == 'v') // validate victim - pStr = str_skip_to_whitespace(pStr); - else if(Command == 'i') // validate int - pStr = str_skip_to_whitespace(pStr); - else if(Command == 'f') // validate float - pStr = str_skip_to_whitespace(pStr); - else if(Command == 's') // validate string + else if(Command == 'v' || Command == 'i' || Command == 'f' || Command == 's') pStr = str_skip_to_whitespace(pStr); if(pStr[0] != 0) // check for end of string @@ -230,6 +231,32 @@ int CConsole::ParseArgs(CResult *pResult, const char *pFormat) pStr++; } + // validate args + if(Command == 'i') + { + // don't validate colors here + if(pfnCallback != &SColorConfigVariable::CommandCallback) + { + int Value; + if(!str_toint(pResult->GetString(pResult->NumArguments() - 1), &Value) || + Value == std::numeric_limits::max() || Value == std::numeric_limits::min()) + { + Error = PARSEARGS_INVALID_INTEGER; + break; + } + } + } + else if(Command == 'f') + { + float Value; + if(!str_tofloat(pResult->GetString(pResult->NumArguments() - 1), &Value) || + Value == std::numeric_limits::max() || Value == std::numeric_limits::min()) + { + Error = PARSEARGS_INVALID_FLOAT; + break; + } + } + if(pVictim) { pResult->SetVictim(pVictim); @@ -487,10 +514,15 @@ void CConsole::ExecuteLineStroked(int Stroke, const char *pStr, int ClientId, bo if(Stroke || IsStrokeCommand) { - if(ParseArgs(&Result, pCommand->m_pParams)) + if(int Error = ParseArgs(&Result, pCommand->m_pParams, pCommand->m_pfnCallback)) { - char aBuf[TEMPCMD_NAME_LENGTH + TEMPCMD_PARAMS_LENGTH + 32]; - str_format(aBuf, sizeof(aBuf), "Invalid arguments. Usage: %s %s", pCommand->m_pName, pCommand->m_pParams); + char aBuf[CMDLINE_LENGTH + 64]; + if(Error == PARSEARGS_INVALID_INTEGER) + str_format(aBuf, sizeof(aBuf), "%s is not a valid integer.", Result.GetString(Result.NumArguments() - 1)); + else if(Error == PARSEARGS_INVALID_FLOAT) + str_format(aBuf, sizeof(aBuf), "%s is not a valid decimal number.", Result.GetString(Result.NumArguments() - 1)); + else + str_format(aBuf, sizeof(aBuf), "Invalid arguments. Usage: %s %s", pCommand->m_pName, pCommand->m_pParams); Print(OUTPUT_LEVEL_STANDARD, "chatresp", aBuf); } else if(m_StoreCommands && pCommand->m_Flags & CFGFLAG_STORE) diff --git a/src/engine/shared/console.h b/src/engine/shared/console.h index 8d8a85231..c3779c6a2 100644 --- a/src/engine/shared/console.h +++ b/src/engine/shared/console.h @@ -115,7 +115,7 @@ class CConsole : public IConsole const char *GetString(unsigned Index) const override; int GetInteger(unsigned Index) const override; float GetFloat(unsigned Index) const override; - ColorHSLA GetColor(unsigned Index, bool Light) const override; + std::optional GetColor(unsigned Index, bool Light) const override; void RemoveArgument(unsigned Index) override { @@ -144,7 +144,16 @@ class CConsole : public IConsole }; int ParseStart(CResult *pResult, const char *pString, int Length); - int ParseArgs(CResult *pResult, const char *pFormat); + + enum + { + PARSEARGS_OK = 0, + PARSEARGS_MISSING_VALUE, + PARSEARGS_INVALID_INTEGER, + PARSEARGS_INVALID_FLOAT, + }; + + int ParseArgs(CResult *pResult, const char *pFormat, FCommandCallback pfnCallback = 0); /* this function will set pFormat to the next parameter (i,s,r,v,?) it contains and diff --git a/src/rust-bridge/cpp/console.cpp b/src/rust-bridge/cpp/console.cpp index fb5e3cbd9..eeaab723b 100644 --- a/src/rust-bridge/cpp/console.cpp +++ b/src/rust-bridge/cpp/console.cpp @@ -109,8 +109,8 @@ void cxxbridge1$IConsole_IResult$GetString(const ::IConsole_IResult &self, ::std } void cxxbridge1$IConsole_IResult$GetColor(const ::IConsole_IResult &self, ::std::uint32_t Index, bool Light, ::ColorHSLA *return$) noexcept { - ::ColorHSLA (::IConsole_IResult::*GetColor$)(::std::uint32_t, bool) const = &::IConsole_IResult::GetColor; - new (return$) ::ColorHSLA((self.*GetColor$)(Index, Light)); + std::optional<::ColorHSLA> (::IConsole_IResult::*GetColor$)(::std::uint32_t, bool) const = &::IConsole_IResult::GetColor; + new(return$)::ColorHSLA((self.*GetColor$)(Index, Light).value_or(::ColorHSLA(0, 0, 0))); } ::std::int32_t cxxbridge1$IConsole_IResult$NumArguments(const ::IConsole_IResult &self) noexcept {