Bug 398154 - kpmcore starts external process dd
Summary: kpmcore starts external process dd
Status: RESOLVED FIXED
Alias: None
Product: partitionmanager
Classification: Applications
Component: general (show other bugs)
Version: Git
Platform: Other All
: NOR normal
Target Milestone: ---
Assignee: Andrius Štikonas
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-02 12:20 UTC by Pali Rohár
Modified: 2018-11-29 22:32 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pali Rohár 2018-09-02 12:20:52 UTC
It does not make any sense to use external process dd. Qt provides QFile class for reading and writing to files.

$ git grep '"dd"'
src/fs/fat12.cpp:    m_UpdateUUID = findExternal(QStringLiteral("dd")) ? cmdSupportFileSystem : cmdSupportNone;
src/fs/fat12.cpp:    ExternalCommand cmd(report, QStringLiteral("dd"), { QStringLiteral("of=") + deviceNode , QStringLiteral("bs=1"), QStringLiteral("count=4"), QStringLiteral("seek=39") });
src/fs/fat16.cpp:    m_UpdateUUID = findExternal(QStringLiteral("dd")) ? cmdSupportFileSystem : cmdSupportNone;
src/fs/fat32.cpp:    ExternalCommand cmd(report, QStringLiteral("dd"), { QStringLiteral("of=") + deviceNode, QStringLiteral("bs=1"), QStringLiteral("count=4"), QStringLiteral("seek=67") });
src/fs/ntfs.cpp:    ExternalCommand cmd(report, QStringLiteral("dd"), { QStringLiteral("of=") + deviceNode , QStringLiteral("bs=1"), QStringLiteral("count=4"), QStringLiteral("seek=28") });
src/fs/ntfs.cpp:    ExternalCommand cmd2(report, QStringLiteral("dd"), { QStringLiteral("of=") + deviceNode , QStringLiteral("bs=1"), QStringLiteral("count=4"), QStringLiteral("seek=") + QString::number(pos) });
src/plugins/sfdisk/sfdiskbackend.cpp:        ExternalCommand ddCommand(QStringLiteral("dd"),
src/util/externalcommand_whitelist.h:QStringLiteral("dd"),
Comment 1 Andrius Štikonas 2018-09-02 16:42:10 UTC
That's temporary in my plan. I want to eventually (before release) remove both "dd" and "mv"

Simple QFile wouldn't work as it wouldn't have root permissions. It has to be done by KAuth helper. Right now we have 2 KAuth helpers:
 1) externalcommand: running system command (from external command whitelist)
 2) copyblocks: copy arbitrary data from one file/block device to another (this one uses QFile under the hood.

So we can migrate to the second helper with KAuth.


P.S. there is also a bit of misuse in the way we use KAuth. We just run KAuth helper and do not return from it until KPM finishes its job. This is done so that the user would not have to approve every single external command request separately (which would be impractical, nobody would attempt to enter password like 20 times in a row).

That's why it might make sense to harder the helper a bit. E.g. don't allow copyblocks to write to /etc/ or /usr.
Comment 2 Andrius Štikonas 2018-10-30 23:30:33 UTC
Fixed for src/plugins/sfdisk/sfdiskbackend.cpp (it used dd if=)

fat/ntfs boot sectors are not fixed yet (this uses dd of=). It's slighly harder than previous case but hopefully not significantly (might need to modify signature of copyblocks function in kauth helper)
Comment 3 Andrius Štikonas 2018-11-25 20:51:27 UTC
Git commit 09e4d47e0711a800be858fdd6691f59838bc3003 by Andrius Štikonas.
Committed on 25/11/2018 at 20:50.
Pushed by stikonas into branch 'master'.

Do not use external process dd.

M  +3    -5    src/fs/fat12.cpp
M  +5    -6    src/fs/fat32.cpp
M  +2    -5    src/fs/ntfs.cpp
M  +49   -0    src/util/externalcommand.cpp
M  +1    -0    src/util/externalcommand.h
M  +0    -1    src/util/externalcommand_whitelist.h
M  +29   -1    src/util/externalcommandhelper.cpp
M  +1    -0    src/util/externalcommandhelper.h

https://commits.kde.org/kpmcore/09e4d47e0711a800be858fdd6691f59838bc3003
Comment 4 Andrius Štikonas 2018-11-29 22:32:54 UTC
Git commit 80f5a32dc04c4150d3b32c46526b6489f8fa853c by Andrius Štikonas.
Committed on 29/11/2018 at 22:32.
Pushed by stikonas into branch 'master'.

Remove remaining cases of call to dd binary.

M  +1    -1    src/fs/fat12.cpp
M  +1    -1    src/fs/fat16.cpp
M  +1    -4    src/fs/ntfs.cpp
M  +1    -1    src/util/externalcommandhelper.cpp
M  +1    -1    src/util/externalcommandhelper.h

https://commits.kde.org/kpmcore/80f5a32dc04c4150d3b32c46526b6489f8fa853c