Skip to content

Commit

Permalink
bug fixes: fix #44, #45 and #66
Browse files Browse the repository at this point in the history
- (44) fixes windows UI issue
- (45) fixes global UI issue
- (66) fix crash on invalid ID
  • Loading branch information
m-simonelli authored Mar 25, 2021
1 parent 3fa5766 commit 1e83d96
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 26 deletions.
14 changes: 14 additions & 0 deletions src/libtego/include/tego/tego.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ void tego_ed25519_public_key_from_ed25519_private_key(
const tego_ed25519_private_key_t* privateKey,
tego_error_t** error);

/*
* Checks if a service id string is valid per tor rend spec:
* https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt
*
* @param serviceIdString : string containing the v3 service id to be validated
* @param serviceIdStringLength : length of serviceIdString not counting the
* null terminator
* @param error : filled on error
*/
tego_bool_t tego_v3_onion_service_id_string_is_valid(
const char* serviceIdString,
size_t serviceIdStringLength,
tego_error_t** error);

/*
* Construct a service id object from string. Validates
* the checksum and version byte per spec:
Expand Down
2 changes: 1 addition & 1 deletion src/libtego/include/tego/tego.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

// libtego
#include <tego/utilities.hpp>
// #define ENABLE_TEGO_LOGGER
//#define ENABLE_TEGO_LOGGER
#include <tego/logger.hpp>

namespace tego
Expand Down
4 changes: 2 additions & 2 deletions src/libtego/source/core/ContactIDValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

// multiple consumers of this regex object seems to cause thread contention issues
// and segfaults, so make it thread_local to sidestep the issue for now
static thread_local QRegularExpression regex(QStringLiteral("(torsion|ricochet):([a-z2-7]{56})"));
static thread_local QRegularExpression regex(QStringLiteral("ricochet:([a-z2-7]{56})"));

ContactIDValidator::ContactIDValidator(QObject *parent)
: QRegularExpressionValidator(parent)
Expand Down Expand Up @@ -96,7 +96,7 @@ QString ContactIDValidator::hostnameFromID(const QString &ID)
if (!match.hasMatch())
return QString();

return match.captured(2) + QStringLiteral(".onion");
return match.captured(1) + QStringLiteral(".onion");
}

QString ContactIDValidator::idFromHostname(const QString &hostname)
Expand Down
70 changes: 53 additions & 17 deletions src/libtego/source/ed25519.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,33 +74,54 @@ tego_v3_onion_service_id::tego_v3_onion_service_id(
TEGO_THROW_IF_FALSE(serviceIdStringLength >= TEGO_V3_ONION_SERVICE_ID_LENGTH);

std::string_view serviceIdView(serviceIdString, TEGO_V3_ONION_SERVICE_ID_LENGTH);
uint8_t rawServiceId[TEGO_V3_ONION_SERVICE_ID_RAW_SIZE] = {0};
TEGO_THROW_IF_FALSE(is_valid(serviceIdView));
// copy to our internal buffer
std::copy(
std::begin(serviceIdView),
std::end(serviceIdView),
this->data);
}

tego_bool_t tego_v3_onion_service_id::is_valid(
std::string_view &serviceIdString)
{
if (serviceIdString.size() != TEGO_V3_ONION_SERVICE_ID_LENGTH)
{
return TEGO_FALSE;
}

uint8_t decodedServiceId[TEGO_V3_ONION_SERVICE_ID_RAW_SIZE] = {0};

// base32 decode service serviceId
const auto bytesDecoded = ::base32_decode(
reinterpret_cast<char*>(rawServiceId),
sizeof(rawServiceId),
serviceIdView.data(),
serviceIdView.size());
TEGO_THROW_IF_FALSE(bytesDecoded == sizeof(rawServiceId));

// verify correct version byte
TEGO_THROW_IF_FALSE(rawServiceId[TEGO_V3_ONION_SERVICE_ID_VERSION_OFFSET] == 0x03);
reinterpret_cast<char *>(decodedServiceId),
sizeof(decodedServiceId),
serviceIdString.data(),
serviceIdString.size());

// check successful base32 decode and correct version byte
if (bytesDecoded != sizeof(decodedServiceId) ||
decodedServiceId[TEGO_V3_ONION_SERVICE_ID_VERSION_OFFSET] != 0x03)
{
return TEGO_FALSE;
}

// first part of the rawServiceId is the publicKey
auto& rawPublicKey = reinterpret_cast<uint8_t (&)[ED25519_PUBKEY_LEN]>(rawServiceId);
auto& rawPublicKey = reinterpret_cast<uint8_t (&)[ED25519_PUBKEY_LEN]>(decodedServiceId);

// calculate the truncated checksum for the public key
uint8_t truncatedChecksum[TEGO_V3_ONION_SERVICE_ID_CHECKSUM_SIZE] = {0};
tego::truncated_checksum_from_ed25519_public_key(truncatedChecksum, rawPublicKey);

// verify the first two bytes of checksum in service id match our calculated checksum
TEGO_THROW_IF_FALSE(
rawServiceId[TEGO_V3_ONION_SERVICE_ID_CHECKSUM_OFFSET ] == truncatedChecksum[0] &&
rawServiceId[TEGO_V3_ONION_SERVICE_ID_CHECKSUM_OFFSET + 1] == truncatedChecksum[1]);
auto validChecksum = decodedServiceId[TEGO_V3_ONION_SERVICE_ID_CHECKSUM_OFFSET ] == truncatedChecksum[0] &&
decodedServiceId[TEGO_V3_ONION_SERVICE_ID_CHECKSUM_OFFSET + 1] == truncatedChecksum[1];

if (!validChecksum)
{
return TEGO_FALSE;
}

// copy to our internal buffer
std::copy(std::begin(serviceIdView), std::end(serviceIdView), this->data);
return TEGO_TRUE;
}

//
Expand Down Expand Up @@ -206,6 +227,21 @@ extern "C"
}, error);
}

tego_bool_t tego_v3_onion_service_id_string_is_valid(
const char* serviceIdString,
size_t serviceIdStringLength,
tego_error_t** error)
{
return tego::translateExceptions([&]() -> tego_bool_t
{
TEGO_THROW_IF_NULL(serviceIdString);
TEGO_THROW_IF_FALSE(serviceIdStringLength >= TEGO_V3_ONION_SERVICE_ID_LENGTH);

std::string_view serviceIdView(serviceIdString, TEGO_V3_ONION_SERVICE_ID_LENGTH);
return tego_v3_onion_service_id::is_valid(serviceIdView);
}, error, TEGO_FALSE);
}

void tego_v3_onion_service_id_from_string(
tego_v3_onion_service_id_t** out_serviceId,
const char* serviceIdString,
Expand Down Expand Up @@ -426,4 +462,4 @@ extern "C"
return TEGO_FALSE;
}, error, TEGO_FALSE);
}
}
}
1 change: 1 addition & 0 deletions src/libtego/source/ed25519.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct tego_v3_onion_service_id
{
tego_v3_onion_service_id() = default;
tego_v3_onion_service_id(const char* serviceIdString, size_t serviceIdStringLength);
static tego_bool_t is_valid(std::string_view &serviceIdString);

char data[TEGO_V3_ONION_SERVICE_ID_SIZE] = {0};
};
20 changes: 18 additions & 2 deletions src/libtego_ui/shims/ContactIDValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@ namespace shims
{
emit failed();
}
else
{
emit success(); // removes the popup when the id returns to being valid
}
return re;
}

if (matchingContact(text) || matchesIdentity(text))
if (!isValidID(text) || matchingContact(text) || matchesIdentity(text))
{
emit failed();
return QValidator::Invalid;
}

emit success();
return re;
}

Expand Down Expand Up @@ -69,4 +74,15 @@ namespace shims

return utf8Text == utf8ServiceId;
}
}

bool ContactIDValidator::isValidID(const QString &serviceID) const
{
auto strippedID = serviceID.mid(tego::static_strlen("ricochet:"));
logger::println("strippedID : {}", strippedID.toUtf8().constData(), strippedID.size());

bool valid = tego_v3_onion_service_id_string_is_valid(strippedID.toUtf8().constData(), strippedID.size(), nullptr) == TEGO_TRUE;
logger::println("valid: {}", valid);

return valid;
}
}
2 changes: 2 additions & 0 deletions src/libtego_ui/shims/ContactIDValidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ namespace shims
virtual void fixup(QString &text) const;
virtual State validate(QString &text, int &pos) const;

Q_INVOKABLE bool isValidID(const QString &serviceID) const;
Q_INVOKABLE shims::ContactUser *matchingContact(const QString &text) const;
Q_INVOKABLE bool matchesIdentity(const QString &text) const;
signals:
void failed() const;
void success() const;
};
}
7 changes: 7 additions & 0 deletions src/libtego_ui/shims/ContactsManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ namespace shims
contactID, nickname, myNickname, message);

auto serviceId = contactID.mid(tego::static_strlen("ricochet:")).toUtf8();

// check that the service id is valid before anything else
if (tego_v3_onion_service_id_string_is_valid(serviceId.constData(), serviceId.size(), nullptr) != TEGO_TRUE)
{
return nullptr;
}

auto shimContact = this->addContact(serviceId, nickname);

auto userId = shimContact->toTegoUserId();
Expand Down
2 changes: 1 addition & 1 deletion src/libtego_ui/ui/qml/AddContactDialog.qml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import QtQuick.Layouts 1.0

ApplicationWindow {
id: addContactWindow
width: 620
width: 640
height: 300
minimumWidth: width
maximumWidth: width
Expand Down
9 changes: 9 additions & 0 deletions src/libtego_ui/ui/qml/ContactIDField.qml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ FocusScope {
//: Error message showed when user attempts to add a contact already in their contact list
errorBubble.show(qsTr("<b>%1</b> is already your contact").arg(Utils.htmlEscaped(contact.nickname)))
}
else if (!isValidID(field.text))
{
//: Error message showed when the id doesn't comply with spec https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt
errorBubble.show(qsTr("This ID is invalid"));
}
else if (matchesIdentity(field.text))
{
//: Error message showed when user attempts to add themselves as a contact in their contact list
Expand All @@ -50,6 +55,10 @@ FocusScope {
errorBubble.show(qsTr("Enter an ID starting with <b>ricochet:</b>"))
}
}

onSuccess: {
errorBubble.clear()
}
}

Bubble {
Expand Down
2 changes: 1 addition & 1 deletion src/libtego_ui/ui/qml/ContactRequestDialog.qml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import QtQuick.Layouts 1.0

ApplicationWindow {
id: contactRequestDialog
width: 350
width: 640
height: 200
minimumWidth: width
maximumWidth: width
Expand Down
4 changes: 2 additions & 2 deletions src/libtego_ui/ui/qml/PreferencesDialog.qml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import im.ricochet 1.0

ApplicationWindow {
id: preferencesWindow
width: 550
minimumWidth: 550
width: 820
minimumWidth: 820
height: 400
minimumHeight: 400
title: qsTr("Ricochet Preferences")
Expand Down
1 change: 1 addition & 0 deletions src/libtego_ui/ui/qml/TorConfigurationPage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ Column {
//: Button label for connecting to tor
text: qsTr("Connect")
isDefault: true
enabled: setup.proxyType ? (proxyAddressField.text && proxyPortField.text) : true
onClicked: {
setup.save()
}
Expand Down

0 comments on commit 1e83d96

Please sign in to comment.