fix: security hardening and code quality improvements
Build hardening: - Add -fstack-protector-strong, -D_FORTIFY_SOURCE=2, PIE, full RELRO - Enable overflow-checks in Rust release profile Rust backend: - Return null (not panic) if Tokio runtime or QobuzClient init fails - Strip null bytes in FFI JSON callback to prevent CString panics - Document MD5 and password-in-query as Qobuz API constraints C++ frontend: - Validate JSON document before accessing fields in onEvent() - Handle null backend pointer from failed init - Set biography label to PlainText and decode HTML entities to prevent rendering injected content from API responses - Clamp slider position and guard negative durations - Use qint64 for duration formatting to avoid int truncation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -88,9 +88,18 @@ if (UNIX AND NOT APPLE)
|
|||||||
target_link_libraries(qobuz-qt PRIVATE asound)
|
target_link_libraries(qobuz-qt PRIVATE asound)
|
||||||
endif ()
|
endif ()
|
||||||
|
|
||||||
# Compiler warnings
|
# Compiler warnings + hardening
|
||||||
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
|
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
|
||||||
target_compile_options(qobuz-qt PRIVATE -Wall -Wextra -Wno-unused-parameter)
|
target_compile_options(qobuz-qt PRIVATE
|
||||||
|
-Wall -Wextra -Wno-unused-parameter
|
||||||
|
-fstack-protector-strong
|
||||||
|
-D_FORTIFY_SOURCE=2
|
||||||
|
-fPIE
|
||||||
|
)
|
||||||
|
target_link_options(qobuz-qt PRIVATE
|
||||||
|
-pie
|
||||||
|
-Wl,-z,relro,-z,now
|
||||||
|
)
|
||||||
endif ()
|
endif ()
|
||||||
|
|
||||||
# D-Bus
|
# D-Bus
|
||||||
|
|||||||
@@ -31,3 +31,4 @@ toml = "0.8"
|
|||||||
[profile.release]
|
[profile.release]
|
||||||
lto = "thin"
|
lto = "thin"
|
||||||
opt-level = 3
|
opt-level = 3
|
||||||
|
overflow-checks = true
|
||||||
|
|||||||
@@ -60,6 +60,8 @@ impl QobuzClient {
|
|||||||
.as_secs()
|
.as_secs()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Compute the request signature required by the Qobuz API.
|
||||||
|
/// NOTE: MD5 is mandated by the Qobuz API protocol — not our choice.
|
||||||
fn request_sig(&self, method: &str, params: &mut Vec<(&str, String)>, ts: u64) -> String {
|
fn request_sig(&self, method: &str, params: &mut Vec<(&str, String)>, ts: u64) -> String {
|
||||||
params.sort_by_key(|(k, _)| *k);
|
params.sort_by_key(|(k, _)| *k);
|
||||||
let mut s = method.replace('/', "");
|
let mut s = method.replace('/', "");
|
||||||
@@ -116,6 +118,7 @@ impl QobuzClient {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// NOTE: Qobuz API requires credentials as GET query params — not our choice.
|
||||||
async fn oauth2_login(&mut self, email: &str, password: &str) -> Result<OAuthLoginResponse> {
|
async fn oauth2_login(&mut self, email: &str, password: &str) -> Result<OAuthLoginResponse> {
|
||||||
let ts = Self::ts();
|
let ts = Self::ts();
|
||||||
let mut sign_params: Vec<(&str, String)> = vec![
|
let mut sign_params: Vec<(&str, String)> = vec![
|
||||||
|
|||||||
@@ -96,7 +96,9 @@ pub struct Backend(BackendInner);
|
|||||||
// ---------- Helpers ----------
|
// ---------- Helpers ----------
|
||||||
|
|
||||||
fn call_cb(cb: EventCallback, ud: SendPtr, ev: c_int, json: &str) {
|
fn call_cb(cb: EventCallback, ud: SendPtr, ev: c_int, json: &str) {
|
||||||
let cstr = CString::new(json).unwrap_or_else(|_| CString::new("{}").unwrap());
|
// Strip null bytes that would cause CString::new to fail
|
||||||
|
let safe = json.replace('\0', "");
|
||||||
|
let cstr = CString::new(safe).unwrap_or_else(|_| CString::new("{}").unwrap());
|
||||||
unsafe { cb(ud.0, ev, cstr.as_ptr()) };
|
unsafe { cb(ud.0, ev, cstr.as_ptr()) };
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -119,8 +121,14 @@ pub unsafe extern "C" fn qobuz_backend_new(
|
|||||||
event_cb: EventCallback,
|
event_cb: EventCallback,
|
||||||
userdata: *mut c_void,
|
userdata: *mut c_void,
|
||||||
) -> *mut Backend {
|
) -> *mut Backend {
|
||||||
let rt = Runtime::new().expect("tokio runtime");
|
let rt = match Runtime::new() {
|
||||||
let client = Arc::new(Mutex::new(QobuzClient::new().expect("QobuzClient")));
|
Ok(r) => r,
|
||||||
|
Err(_) => return std::ptr::null_mut(),
|
||||||
|
};
|
||||||
|
let client = match QobuzClient::new() {
|
||||||
|
Ok(c) => Arc::new(Mutex::new(c)),
|
||||||
|
Err(_) => return std::ptr::null_mut(),
|
||||||
|
};
|
||||||
let player = Player::new();
|
let player = Player::new();
|
||||||
|
|
||||||
Box::into_raw(Box::new(Backend(BackendInner {
|
Box::into_raw(Box::new(Backend(BackendInner {
|
||||||
|
|||||||
@@ -8,6 +8,10 @@ QobuzBackend::QobuzBackend(QObject *parent)
|
|||||||
: QObject(parent)
|
: QObject(parent)
|
||||||
{
|
{
|
||||||
m_backend = qobuz_backend_new(&QobuzBackend::eventTrampoline, this);
|
m_backend = qobuz_backend_new(&QobuzBackend::eventTrampoline, this);
|
||||||
|
if (!m_backend) {
|
||||||
|
qCritical("Failed to initialize Qobuz backend");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
m_positionTimer = new QTimer(this);
|
m_positionTimer = new QTimer(this);
|
||||||
m_positionTimer->setInterval(50);
|
m_positionTimer->setInterval(50);
|
||||||
@@ -194,7 +198,12 @@ void QobuzBackend::onPositionTick()
|
|||||||
|
|
||||||
void QobuzBackend::onEvent(int eventType, const QString &json)
|
void QobuzBackend::onEvent(int eventType, const QString &json)
|
||||||
{
|
{
|
||||||
const QJsonObject obj = QJsonDocument::fromJson(json.toUtf8()).object();
|
const QJsonDocument doc = QJsonDocument::fromJson(json.toUtf8());
|
||||||
|
if (!doc.isObject()) {
|
||||||
|
emit error(tr("Malformed response from backend"));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const QJsonObject obj = doc.object();
|
||||||
|
|
||||||
switch (eventType) {
|
switch (eventType) {
|
||||||
case EV_LOGIN_OK:
|
case EV_LOGIN_OK:
|
||||||
|
|||||||
@@ -294,7 +294,8 @@ void TrackListModel::sort(int column, Qt::SortOrder order)
|
|||||||
|
|
||||||
QString TrackListModel::formatDuration(qint64 secs)
|
QString TrackListModel::formatDuration(qint64 secs)
|
||||||
{
|
{
|
||||||
const int m = static_cast<int>(secs / 60);
|
if (secs < 0) secs = 0;
|
||||||
const int s = static_cast<int>(secs % 60);
|
const qint64 m = secs / 60;
|
||||||
|
const qint64 s = secs % 60;
|
||||||
return QStringLiteral("%1:%2").arg(m).arg(s, 2, 10, QLatin1Char('0'));
|
return QStringLiteral("%1:%2").arg(m).arg(s, 2, 10, QLatin1Char('0'));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -133,15 +133,22 @@ void ArtistView::setArtist(const QJsonObject &artist)
|
|||||||
// artist/page: name is {"display": "..."}
|
// artist/page: name is {"display": "..."}
|
||||||
m_nameLabel->setText(artist["name"].toObject()["display"].toString());
|
m_nameLabel->setText(artist["name"].toObject()["display"].toString());
|
||||||
|
|
||||||
// biography.content is HTML — strip tags for the summary label
|
// biography.content is HTML — strip tags for safe plain-text display
|
||||||
const QString bioHtml = artist["biography"].toObject()["content"].toString();
|
const QString bioHtml = artist["biography"].toObject()["content"].toString();
|
||||||
if (!bioHtml.isEmpty()) {
|
if (!bioHtml.isEmpty()) {
|
||||||
// Remove HTML tags for plain-text display
|
|
||||||
QString plain = bioHtml;
|
QString plain = bioHtml;
|
||||||
|
// Strip HTML entities and tags to prevent rendering injected content
|
||||||
plain.remove(QRegularExpression(QStringLiteral("<[^>]*>")));
|
plain.remove(QRegularExpression(QStringLiteral("<[^>]*>")));
|
||||||
|
plain.replace(QStringLiteral("&"), QStringLiteral("&"));
|
||||||
|
plain.replace(QStringLiteral("<"), QStringLiteral("<"));
|
||||||
|
plain.replace(QStringLiteral(">"), QStringLiteral(">"));
|
||||||
|
plain.replace(QStringLiteral("""), QStringLiteral("\""));
|
||||||
|
plain.replace(QStringLiteral("'"), QStringLiteral("'"));
|
||||||
|
plain.replace(QStringLiteral(" "), QStringLiteral(" "));
|
||||||
plain = plain.trimmed();
|
plain = plain.trimmed();
|
||||||
|
m_bioLabel->setTextFormat(Qt::PlainText);
|
||||||
m_bioLabel->setText(plain);
|
m_bioLabel->setText(plain);
|
||||||
m_bioLabel->setVisible(true);
|
m_bioLabel->setVisible(!plain.isEmpty());
|
||||||
} else {
|
} else {
|
||||||
m_bioLabel->setVisible(false);
|
m_bioLabel->setVisible(false);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -150,8 +150,8 @@ void MainToolBar::setCurrentTrack(const QJsonObject &track)
|
|||||||
void MainToolBar::updateProgress(quint64 position, quint64 duration)
|
void MainToolBar::updateProgress(quint64 position, quint64 duration)
|
||||||
{
|
{
|
||||||
if (m_seeking) return;
|
if (m_seeking) return;
|
||||||
const int sliderPos = duration > 0
|
const int sliderPos = (duration > 0)
|
||||||
? static_cast<int>(position * 1000 / duration) : 0;
|
? static_cast<int>(qMin(position * 1000 / duration, quint64(1000))) : 0;
|
||||||
m_progress->blockSignals(true);
|
m_progress->blockSignals(true);
|
||||||
m_progress->setValue(sliderPos);
|
m_progress->setValue(sliderPos);
|
||||||
m_progress->blockSignals(false);
|
m_progress->blockSignals(false);
|
||||||
|
|||||||
Reference in New Issue
Block a user