From 91dd3d3d4ae14451552afde33a5d99397df5adee Mon Sep 17 00:00:00 2001 From: Fuzzbawls Date: Sun, 15 May 2022 23:20:59 -0700 Subject: [PATCH 1/5] GUI: Don't hard cap proposal name/URL input length Let the validation methods give length warnings --- src/qt/pivx/forms/createproposaldialog.ui | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/qt/pivx/forms/createproposaldialog.ui b/src/qt/pivx/forms/createproposaldialog.ui index 884a3f7135457..81e67b3f53ad4 100644 --- a/src/qt/pivx/forms/createproposaldialog.ui +++ b/src/qt/pivx/forms/createproposaldialog.ui @@ -774,11 +774,7 @@ - - - 20 - - + @@ -795,11 +791,7 @@ - - - 64 - - + From efbc4206c6624947e92160ae5e4bde503a7bddef Mon Sep 17 00:00:00 2001 From: Fuzzbawls Date: Sun, 15 May 2022 23:42:45 -0700 Subject: [PATCH 2/5] GUI: Sanitize Proposal Name/URL input fields and validate This matches the behavior provided by the RPC interface to prevent illegal characters from being included in either a proposal's name or URL --- src/qt/pivx/governancemodel.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/qt/pivx/governancemodel.cpp b/src/qt/pivx/governancemodel.cpp index 631102a3188ba..21c1b91a9b1c8 100644 --- a/src/qt/pivx/governancemodel.cpp +++ b/src/qt/pivx/governancemodel.cpp @@ -182,16 +182,24 @@ std::vector GovernanceModel::getLocalMNsVotesForProposal(const Proposa OperationResult GovernanceModel::validatePropName(const QString& name) const { - if (name.toUtf8().size() > (int)PROP_NAME_MAX_SIZE) { // limit - return {false, _("Invalid name, maximum size exceeded")}; + std::string strName = SanitizeString(name.toStdString()); + if (strName != name.toStdString()) { // invalid characters + return {false, _("Invalid name, invalid characters")}; + } + if (strName.size() > (int)PROP_NAME_MAX_SIZE) { // limit + return {false, strprintf(_("Invalid name, maximum size of %d exceeded"), PROP_NAME_MAX_SIZE)}; } return {true}; } OperationResult GovernanceModel::validatePropURL(const QString& url) const { + std::string strURL = SanitizeString(url.toStdString()); + if (strURL != url.toStdString()) { + return {false, _("Invalid URL, invalid characters")}; + } std::string strError; - return {validateURL(url.toStdString(), strError, PROP_URL_MAX_SIZE), strError}; + return {validateURL(strURL, strError, PROP_URL_MAX_SIZE), strError}; } OperationResult GovernanceModel::validatePropAmount(CAmount amount) const From 76898cb79e1b17bfa9bf619533cd0698fee63057 Mon Sep 17 00:00:00 2001 From: Fuzzbawls Date: Mon, 16 May 2022 07:24:34 -0700 Subject: [PATCH 3/5] [Cleanup] Pass strURL via reference and make const in validateURL --- src/utilstrencodings.cpp | 4 ++-- src/utilstrencodings.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 78c8aafe7a5d1..54679d1f19137 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -38,13 +38,13 @@ std::string SanitizeString(const std::string& str, int rule) return strResult; } -bool validateURL(std::string strURL) +bool validateURL(const std::string& strURL) { std::string strErr; return validateURL(strURL, strErr); } -bool validateURL(std::string strURL, std::string& strErr, unsigned int maxSize) +bool validateURL(const std::string& strURL, std::string& strErr, unsigned int maxSize) { // Check URL size if (strURL.size() > maxSize) { diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index 766eaa507f320..4a27ac2d8cd1c 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -49,8 +49,8 @@ std::string SanitizeString(const std::string& str, int rule = SAFE_CHARS_DEFAULT * @param[in] maxSize An unsigned int, defaulted to 64, to restrict the length * @return A bool, true if valid, false if not (reason in strErr) */ -bool validateURL(std::string strURL, std::string& strErr, unsigned int maxSize = 64); -bool validateURL(std::string strURL); +bool validateURL(const std::string& strURL, std::string& strErr, unsigned int maxSize = 64); +bool validateURL(const std::string& strURL); std::vector ParseHex(const char* psz); std::vector ParseHex(const std::string& str); From 54c0d10dd16f54f83b0d23a1bd0b213aac8844ba Mon Sep 17 00:00:00 2001 From: Fuzzbawls Date: Mon, 16 May 2022 07:27:27 -0700 Subject: [PATCH 4/5] [Cleanup] Use raw string literal in validateURL clang-tidy: can use a raw string literal instead of an escaped string for better readability. --- src/utilstrencodings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index 54679d1f19137..1edea071d61dd 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -53,7 +53,7 @@ bool validateURL(const std::string& strURL, std::string& strErr, unsigned int ma } // Validate URL - std::regex url_regex("^(https?)://[^\\s/$.?#][^\\s]*[^\\s/.]\\.[^\\s/.][^\\s]*[^\\s.]$"); + std::regex url_regex(R"(^(https?)://[^\s/$.?#][^\s]*[^\s/.]\.[^\s/.][^\s]*[^\s.]$)"); if (!std::regex_match(strURL, url_regex)) { strErr = "Invalid URL"; return false; From dc11219f25e01c702240194e1401be188c688ae6 Mon Sep 17 00:00:00 2001 From: Fuzzbawls Date: Mon, 16 May 2022 07:33:46 -0700 Subject: [PATCH 5/5] Future: Introduce future function to validate proposal strings Future implementation of string validation on the network level for proposal name/URL --- src/budget/budgetproposal.cpp | 15 +++++++++++++++ src/budget/budgetproposal.h | 1 + 2 files changed, 16 insertions(+) diff --git a/src/budget/budgetproposal.cpp b/src/budget/budgetproposal.cpp index 08fa76a00774f..e8db19f1572e7 100644 --- a/src/budget/budgetproposal.cpp +++ b/src/budget/budgetproposal.cpp @@ -6,6 +6,7 @@ #include "budget/budgetproposal.h" #include "chainparams.h" #include "script/standard.h" +#include "utilstrencodings.h" CBudgetProposal::CBudgetProposal(): nAllotted(0), @@ -145,6 +146,20 @@ bool CBudgetProposal::CheckAddress() return true; } +/* TODO: Add this to IsWellFormed() for the next hard-fork + * This will networkly reject malformed proposal names and URLs + */ +bool CBudgetProposal::CheckStrings() +{ + if (strProposalName != SanitizeString(strProposalName)) { + strInvalid = "Proposal name contains illegal characters."; + return false; + } + if (strURL != SanitizeString(strURL)) { + strInvalid = "Proposal URL contains illegal characters."; + } +} + bool CBudgetProposal::IsWellFormed(const CAmount& nTotalBudget) { return CheckStartEnd() && CheckAmount(nTotalBudget) && CheckAddress(); diff --git a/src/budget/budgetproposal.h b/src/budget/budgetproposal.h index a8e47fa4b6045..ce2c6c2706403 100644 --- a/src/budget/budgetproposal.h +++ b/src/budget/budgetproposal.h @@ -42,6 +42,7 @@ class CBudgetProposal bool CheckStartEnd(); bool CheckAmount(const CAmount& nTotalBudget); bool CheckAddress(); + bool CheckStrings(); protected: std::map mapVotes;