Skip to content

Commit e52dfd6

Browse files
fix: Improve error handling in DMDbusHandler and DiskManagerService
- Added checks for the existence of current device paths in DMDbusHandler methods to prevent crashes and log warnings when paths are not found. - Updated DiskManagerService to handle invalid invoker UID retrieval gracefully, ensuring proper authorization checks and logging warnings for failed operations. - Refactored Btrfs size parsing to handle out-of-bounds access more safely. These changes enhance the robustness and reliability of device management operations. task: https://pms.uniontech.com/task-view-386519.html
1 parent cf49376 commit e52dfd6

4 files changed

Lines changed: 56 additions & 23 deletions

File tree

application/partedproxy/dmdbushandler.cpp

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,12 @@ DeviceInfoMap &DMDbusHandler::probDeviceInfo()
290290
PartitionVec DMDbusHandler::getCurDevicePartitionArr()
291291
{
292292
qDebug() << "DMDbusHandler::getCurDevicePartitionArr called.";
293-
return m_deviceMap.find(m_curDevicePath).value().m_partition;
293+
auto it = m_deviceMap.find(m_curDevicePath);
294+
if (it != m_deviceMap.end()) {
295+
return it.value().m_partition;
296+
}
297+
qWarning() << "Current device path not found in map:" << m_curDevicePath;
298+
return PartitionVec();
294299
}
295300

296301
const PartitionInfo &DMDbusHandler::getCurPartititonInfo()
@@ -302,13 +307,25 @@ const PartitionInfo &DMDbusHandler::getCurPartititonInfo()
302307
const DeviceInfo &DMDbusHandler::getCurDeviceInfo()
303308
{
304309
qDebug() << "DMDbusHandler::getCurDeviceInfo called.";
305-
return m_deviceMap.find(m_curDevicePath).value();
310+
auto it = m_deviceMap.find(m_curDevicePath);
311+
if (it != m_deviceMap.end()) {
312+
return it.value();
313+
}
314+
qWarning() << "Current device path not found in map:" << m_curDevicePath;
315+
static const DeviceInfo empty;
316+
return empty;
306317
}
307318

308319
const Sector &DMDbusHandler::getCurDeviceInfoLength()
309320
{
310321
qDebug() << "DMDbusHandler::getCurDeviceInfoLength called.";
311-
return m_deviceMap.find(m_curDevicePath).value().m_length;
322+
auto it = m_deviceMap.find(m_curDevicePath);
323+
if (it != m_deviceMap.end()) {
324+
return it.value().m_length;
325+
}
326+
qWarning() << "Current device path not found in map:" << m_curDevicePath;
327+
static const Sector zero = 0;
328+
return zero;
312329
}
313330

314331
void DMDbusHandler::mount(const QString &mountPath)

service/diskmanagerservice.cpp

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,12 @@ bool DiskManagerService::mount(const QString &mountpath)
152152
return false;
153153
}
154154

155-
QString invokerUid = QString::number(connection().interface()->serviceUid(message().service()).value());
155+
QDBusReply<uint> uidReply = connection().interface()->serviceUid(message().service());
156+
if (!uidReply.isValid()) {
157+
qWarning() << "Failed to get invoker UID for mount, refusing operation";
158+
return false;
159+
}
160+
QString invokerUid = QString::number(uidReply.value());
156161
return m_partedcore->mountAndWriteFstab(mountpath, invokerUid);
157162
}
158163

@@ -502,16 +507,25 @@ bool DiskManagerService::checkAuthorization(void)
502507
QString actionId("com.deepin.pkexec.deepin-diskmanager");
503508
QString serviceName = message().service();
504509

505-
if (serviceName == m_frontEndDBusName ||
506-
connection().interface()->serviceUid(serviceName).value() == 0 ||
507-
PolicyKitHelper::instance()->checkAuthorization(actionId, serviceName)) {
508-
qDebug() << "Authorization granted for service:" << serviceName;
510+
if (serviceName == m_frontEndDBusName) {
511+
qDebug() << "Authorization granted for frontend:" << serviceName;
512+
return true;
513+
}
514+
515+
QDBusReply<uint> uidReply = connection().interface()->serviceUid(serviceName);
516+
if (uidReply.isValid() && uidReply.value() == 0) {
517+
qDebug() << "Authorization granted for root user";
509518
return true;
510-
} else {
511-
qWarning() << "Authorization denied for service:" << serviceName;
512-
sendErrorReply(QDBusError::AccessDenied);
513-
return false;
514519
}
520+
521+
if (PolicyKitHelper::instance()->checkAuthorization(actionId, serviceName)) {
522+
qDebug() << "Authorization granted via Polkit for service:" << serviceName;
523+
return true;
524+
}
525+
526+
qWarning() << "Authorization denied for service:" << serviceName;
527+
sendErrorReply(QDBusError::AccessDenied);
528+
return false;
515529
#endif
516530
}
517531

service/diskoperation/filesystems/btrfs.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -386,12 +386,7 @@ double Btrfs::btrfsSize2Double(QString &str)
386386
double num = numStr.toDouble();
387387
int index = str.indexOf(numStr);
388388
int pos = index + numStr.length();
389-
if (pos < str.length()) {
390-
qDebug() << "pos < str.length()";
391-
char unit = str.at(pos).toLatin1();
392-
393-
}
394-
char unit = str.at(pos).toLatin1();
389+
char unit = (pos < str.length()) ? str.at(pos).toLatin1() : '\0';
395390
Byte_Value mult;
396391
switch (unit) {
397392
case 'K': mult = KIBIBYTE ; break ;

service/diskoperation/luksoperator/luksoperator.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "luksoperator.h"
77
#include "../fsinfo.h"
88
#include <QFile>
9+
#include <QSaveFile>
910
#include <QDir>
1011
#include <QDateTime>
1112
#include <QJsonDocument>
@@ -668,6 +669,7 @@ bool LUKSOperator::wirteCrypttab(LUKS_INFO &luksInfo, bool isMount)
668669
qDebug() << "open /etc/crypttab failed, return false";
669670
return false;
670671
}
672+
QFileDevice::Permissions origPerm = file.permissions();
671673

672674
// read crypttab
673675
bool findflag = false; //目前默认只改第一个发现的uuid findflag 标志位:是否已经查找到uuid
@@ -698,18 +700,23 @@ bool LUKSOperator::wirteCrypttab(LUKS_INFO &luksInfo, bool isMount)
698700
}
699701
file.close();
700702

701-
//write crypttab
702-
if (!file.open(QIODevice::ReadWrite | QIODevice::Truncate)) {
703+
// 原子写入:先写临时文件,再 rename 到 /etc/crypttab,避免 TOCTOU 竞态
704+
QSaveFile saveFile(QStringLiteral("/etc/crypttab"));
705+
if (!saveFile.open(QIODevice::WriteOnly)) {
703706
qDebug() << "open /etc/crypttab for write failed, return false";
704707
return false;
705708
}
706-
707-
QTextStream out(&file);
709+
QTextStream out(&saveFile);
708710
for (int i = 0; i < list.count(); i++) {
709711
out << list.at(i);
710712
}
711713
out.flush();
712-
file.close();
714+
if (!saveFile.commit()) {
715+
qDebug() << "commit /etc/crypttab failed, return false";
716+
return false;
717+
}
718+
// 恢复原文件权限,避免 QSaveFile 使用临时文件导致权限被 umask 改变
719+
QFile::setPermissions(QStringLiteral("/etc/crypttab"), origPerm);
713720
qDebug() << "Successfully created key file:" << filePath;
714721
qDebug() << "LUKSOperator::wirteCrypttab END";
715722
return true;

0 commit comments

Comments
 (0)