Skip to content

Commit

Permalink
fix(chatform): sanitize the toxstring to remove characters after the \0
Browse files Browse the repository at this point in the history
  • Loading branch information
nickolay168 committed Feb 24, 2025
1 parent d284269 commit 8102572
Show file tree
Hide file tree
Showing 65 changed files with 453 additions and 12 deletions.
3 changes: 3 additions & 0 deletions src/chatlog/chatmessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ ChatMessage::Ptr ChatMessage::createChatMessage(const QString& sender, const QSt
QString senderText = sender;

auto textType = Text::NORMAL;
// TRIfA suffixes
text = TextFormatter::processTrifaSuffixes(text, settings.getHideTrifaSuffix());

// smileys
if (settings.getUseEmoticons())
text = smileyPack.smileyfied(text);
Expand Down
23 changes: 23 additions & 0 deletions src/chatlog/textformatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ const QVector<QRegularExpression> URI_WORD_PATTERNS = {
QRegularExpression(QStringLiteral(R"((?<=^|\s)\S*(ed2k://\|file\|\S+))")),
};

const QRegularExpression TRIFA_WRAPPER(QStringLiteral("[[]TRIfA_SUFFIX[]](.+)[[]/TRIfA_SUFFIX[]]$"),
QRegularExpression::DotMatchesEverythingOption);
const QString TRIFA_PLACEHOLDER = QStringLiteral("<font color=#228B22>[...]</font>");

struct MatchingUri
{
Expand Down Expand Up @@ -242,3 +245,23 @@ QString TextFormatter::applyMarkdown(const QString& message, bool showFormatting
}
return result;
}

/**
* @brief Remove or show suffixes added by TRIfA
* @param message Formatting string
* @param hideTrifaSuffix True if suffixes should be replaced by [...]
* @return Copy of message where <TRIfA_SUFFIX> tags are removed and and suffix
* is replaced by [...], or remained as is.
*/
QString TextFormatter::processTrifaSuffixes(const QString& message, bool hideTrifaSuffix)
{
QString result = message;
if (hideTrifaSuffix) {
return result.replace(TRIFA_WRAPPER, TRIFA_PLACEHOLDER);
}
QRegularExpressionMatch match = TRIFA_WRAPPER.match(result);
if (match.hasMatch()) {
result = result.replace(TRIFA_WRAPPER, match.captured(match.lastCapturedIndex()));
}
return result;
}
2 changes: 2 additions & 0 deletions src/chatlog/textformatter.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ namespace TextFormatter {
QString highlightURI(const QString& message);

QString applyMarkdown(const QString& message, bool showFormattingSymbols);

QString processTrifaSuffixes(const QString& message, bool hideTrifaSuffix);
} // namespace TextFormatter
4 changes: 2 additions & 2 deletions src/core/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ void Core::onFriendMessage(Tox* tox, uint32_t friendId, Tox_Message_Type type,
{
std::ignore = tox;
const bool isAction = (type == TOX_MESSAGE_TYPE_ACTION);
const QString msg = ToxString(cMessage, cMessageSize).getQString();
const QString msg = ToxString(cMessage, cMessageSize, true).getQString();
emit static_cast<Core*>(core)->friendMessageReceived(friendId, msg, isAction);
}

Expand Down Expand Up @@ -492,7 +492,7 @@ void Core::onConferenceMessage(Tox* tox, uint32_t conferenceId, uint32_t peerId,
std::ignore = tox;
Core* core = static_cast<Core*>(vCore);
const bool isAction = type == TOX_MESSAGE_TYPE_ACTION;
const QString message = ToxString(cMessage, length).getQString();
const QString message = ToxString(cMessage, length, true).getQString();
emit core->conferenceMessageReceived(conferenceId, peerId, message, isAction);
}

Expand Down
73 changes: 70 additions & 3 deletions src/core/toxstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,61 @@ ToxString::ToxString(QByteArray text)
* @param length Number of bytes to read from the beginning.
*/
ToxString::ToxString(const uint8_t* text, size_t length)
: ToxString(text, length, false)
{
assert(length <= INT_MAX);
string = QByteArray(reinterpret_cast<const char*>(text), length);
}


/**
* @brief Creates a ToxString from the representation used by c-toxcore.
* @param text Pointer to the beginning of the text.
* @param length Number of bytes to read from the beginning.
* @param fix_trifa if true, fix TRIfA messages v3, containing suffix after \0.
*/
ToxString::ToxString(const uint8_t* text, size_t length, bool fix_trifa)
{
if (length > SIZE_MAX) {
qCritical() << "The string has invalid length!";
return;
}
if (fix_trifa) {
// Try to detect TRIfA message v3 format:
// message itself
// \0\0 TOX_MSGV3_GUARD = 2
// random bytes TOX_MSGV3_MSGID_LENGTH = 32
// time stamp TOX_MSGV3_TIMESTAMP_LENGTH = 4
size_t actualLength = 0;
for (actualLength = 0; actualLength < length; actualLength++) {
if (*(text + actualLength) == 0)
break;
}
// Only trim string if its suffix looks like a TRIfA signature.
if (length - actualLength == 2 + 32 + 4 && *(text + actualLength + 1) == 0) {
// Convert the uint8[4] to timestamp to string
std::time_t trifa_timestamp = 0;
for (int i = 4; i > 0; i--) {
trifa_timestamp = trifa_timestamp << 8;
trifa_timestamp += *(text + length - i);
}
string = QByteArray(reinterpret_cast<const char*>(text), actualLength);
// Mark TRIFA suffix to remove it or replace by format symbol
string.append("[TRIfA_SUFFIX]");
// Skip two \0 symbols.
// The string contains random characters and we need to sanitize it.
QByteArray randomBt(reinterpret_cast<const char*>(text + actualLength + 2), 32);
string.append(getQString(randomBt).toLocal8Bit().constData());
// Convert time to string
char strTime[std::size("yyyy-mm-ddThh:mm:ssZ")];
std::strftime(std::data(strTime), std::size(strTime), "%FT%TZ",
std::gmtime(&trifa_timestamp));
string.append(strTime);
string.append("[/TRIfA_SUFFIX]");
} else {
string = QByteArray(reinterpret_cast<const char*>(text), length);
}
} else {
string = QByteArray(reinterpret_cast<const char*>(text), length);
}
}

/**
Expand All @@ -71,10 +123,25 @@ size_t ToxString::size() const
*
* Removes any non-printable characters from the string. This is a defense-in-depth measure to
* prevent some potential security issues caused by bugs in client code or one of its dependencies.
* @return the string with non printable symbols removed.
*/
QString ToxString::getQString() const
{
const auto tainted = QString::fromUtf8(string).toStdU32String();
return getQString(string);
}


/**
* @brief Interpret the string as UTF-8 encoded QString.
*
* Removes any non-printable characters from the string. This is a defense-in-depth measure to
* prevent some potential security issues caused by bugs in client code or one of its dependencies.
* @param stringVal the buffer with bytes.
* @return the string with non printable symbols removed.
*/
QString ToxString::getQString(QByteArray stringVal) const
{
const auto tainted = QString::fromUtf8(stringVal).toStdU32String();
QSet<std::pair<QChar::Category, char32_t>> removed;
std::u32string cleaned;
std::copy_if(tainted.cbegin(), tainted.cend(), std::back_inserter(cleaned), [&removed](char32_t c) {
Expand Down
2 changes: 2 additions & 0 deletions src/core/toxstring.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ class ToxString
explicit ToxString(const QString& text);
explicit ToxString(QByteArray text);
ToxString(const uint8_t* text, size_t length);
ToxString(const uint8_t* text, size_t length, bool fix_trifa);

const uint8_t* data() const;
size_t size() const;
QString getQString() const;
QString getQString(QByteArray stringVal) const;
const QByteArray& getBytes() const;

private:
Expand Down
7 changes: 6 additions & 1 deletion src/model/notificationgenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

namespace {

const QRegularExpression TRIFA_WRAPPER(QStringLiteral("[[]TRIfA_SUFFIX[]](.+)[[]/TRIfA_SUFFIX[]]$"),
QRegularExpression::DotMatchesEverythingOption);

QString generateContent(const QHash<const Conference*, size_t>& conferenceNotifications,
QString lastMessage, const ToxPk& sender)
{
Expand Down Expand Up @@ -51,7 +54,9 @@ NotificationData NotificationGenerator::friendMessageNotification(const Friend*
}

ret.title = f->getDisplayedName();
ret.message = message;
// Make sure we have removed the TRIfA suffix if any.

ret.message = QString(message).remove(TRIFA_WRAPPER);
ret.pixmap = getSenderAvatar(profile, f->getPublicKey());

return ret;
Expand Down
15 changes: 15 additions & 0 deletions src/persistence/settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ void Settings::loadGlobal()
imagePreview = s.value("imagePreview", true).toBool();
chatMaxWindowSize = s.value("chatMaxWindowSize", 100).toInt();
chatWindowChunkSize = s.value("chatWindowChunkSize", 50).toInt();
hideTrifaSuffix = s.value("hideTrifaSuffix", true).toBool();
});

inGroup(s, "Chat", [this, &s] {
Expand Down Expand Up @@ -690,6 +691,7 @@ void Settings::saveGlobal()
s.setValue("statusChangeNotificationEnabled", statusChangeNotificationEnabled);
s.setValue("showConferenceJoinLeaveMessages", showConferenceJoinLeaveMessages);
s.setValue("spellCheckingEnabled", spellCheckingEnabled);
s.setValue("hideTrifaSuffix", hideTrifaSuffix);
});

inGroup(s, "Chat", [this, &s] { //
Expand Down Expand Up @@ -1443,6 +1445,19 @@ void Settings::setChatMessageFont(const QFont& font)
}
}

bool Settings::getHideTrifaSuffix() const
{
const QMutexLocker<QRecursiveMutex> locker(&bigLock);
return hideTrifaSuffix;
}

void Settings::setHideTrifaSuffix(bool hide)
{
if (setVal(hideTrifaSuffix, hide)) {
emit hideTrifaSuffixChanged(hide);
}
}

void Settings::setWidgetData(const QString& uniqueName, const QByteArray& data)
{
bool updated = false;
Expand Down
5 changes: 5 additions & 0 deletions src/persistence/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ public slots:
void dateFormatChanged(const QString& format);
void statusChangeNotificationEnabledChanged(bool enabled);
void spellCheckingEnabledChanged(bool enabled);
void hideTrifaSuffixChanged(bool show);

// Privacy
void typingNotificationChanged(bool enabled);
Expand Down Expand Up @@ -458,6 +459,9 @@ public slots:
bool getSpellCheckingEnabled() const;
void setSpellCheckingEnabled(bool newValue);

bool getHideTrifaSuffix() const;
void setHideTrifaSuffix(bool hide);

// Privacy
bool getTypingNotification() const;
void setTypingNotification(bool enabled);
Expand Down Expand Up @@ -671,6 +675,7 @@ private slots:
bool statusChangeNotificationEnabled;
bool showConferenceJoinLeaveMessages;
bool spellCheckingEnabled;
bool hideTrifaSuffix;

// Privacy
bool typingNotification;
Expand Down
6 changes: 6 additions & 0 deletions src/widget/form/settings/userinterfaceform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ UserInterfaceForm::UserInterfaceForm(SmileyPack& smileyPack_, Settings& settings
bodyUI->chatLogMaxTxt->setValue(settings.getChatMaxWindowSize());
bodyUI->chatLogChunkTxt->setValue(settings.getChatWindowChunkSize());
bodyUI->cbImagePreview->setChecked(settings.getImagePreview());
bodyUI->cbHideTrifaSuffix->setChecked(settings.getHideTrifaSuffix());

bodyUI->cbConferencePosition->setChecked(settings.getConferencePosition());
bodyUI->cbCompactLayout->setChecked(settings.getCompactLayout());
Expand Down Expand Up @@ -399,6 +400,11 @@ void UserInterfaceForm::on_cbImagePreview_stateChanged()
settings.setImagePreview(bodyUI->cbImagePreview->isChecked());
}

void UserInterfaceForm::on_cbHideTrifaSuffix_stateChanged()
{
settings.setHideTrifaSuffix(bodyUI->cbHideTrifaSuffix->isChecked());
}

void UserInterfaceForm::on_chatLogChunkTxt_valueChanged(int value)
{
settings.setChatWindowChunkSize(value);
Expand Down
1 change: 1 addition & 0 deletions src/widget/form/settings/userinterfaceform.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ private slots:
void on_txtChatFontSize_valueChanged(int px);
void on_useNameColors_stateChanged(int value);
void on_cbImagePreview_stateChanged();
void on_cbHideTrifaSuffix_stateChanged();
void on_chatLogMaxTxt_valueChanged(int value);
void on_chatLogChunkTxt_valueChanged(int value);

Expand Down
19 changes: 13 additions & 6 deletions src/widget/form/settings/userinterfacesettings.ui
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
<rect>
<x>0</x>
<y>0</y>
<width>664</width>
<height>993</height>
<width>650</width>
<height>1104</height>
</rect>
</property>
<layout class="QVBoxLayout" name="verticalLayout_4" stretch="0,0,0,0,0,0">
Expand Down Expand Up @@ -167,6 +167,13 @@ Hide formatting characters:
</property>
</widget>
</item>
<item row="3" column="0">
<widget class="QLabel" name="label">
<property name="text">
<string>Chat log:</string>
</property>
</widget>
</item>
<item row="3" column="1">
<widget class="QCheckBox" name="cbImagePreview">
<property name="toolTip">
Expand All @@ -177,7 +184,7 @@ Hide formatting characters:
</property>
</widget>
</item>
<item row="4" column="1">
<item row="5" column="1">
<layout class="QHBoxLayout" name="horizontalLayout_4">
<item>
<widget class="QLabel" name="chatLogMaxLbl">
Expand Down Expand Up @@ -224,10 +231,10 @@ number here may cause the scroll bar to disappear.</string>
</item>
</layout>
</item>
<item row="3" column="0">
<widget class="QLabel" name="label">
<item row="4" column="1">
<widget class="QCheckBox" name="cbHideTrifaSuffix">
<property name="text">
<string>Chat log:</string>
<string>Hide TRIfA suffix</string>
</property>
</widget>
</item>
Expand Down
Loading

0 comments on commit 8102572

Please sign in to comment.