Dangerous call to bash shell allows command injection

[n303p4]

Kiview/src/documentviewer.cpp

Lines 42 to 47 in c065ebe

	std::string command = libreofficeCommand + " --headless --nolockcheck --norestore --convert-to pdf '" + path + "' --outdir " + tempDir;
	QProcess process;
	process.setProgram(QStringLiteral("setsid"));
	process.setArguments( QStringList() << QStringLiteral("bash") << QStringLiteral("-c") << QString::fromStdString(command) );

bash is called here and there's no input sanitization done on the file path.

As an example, if a filename includes the string ' & (cd && rm -rf *) & ', and an unwitting user previews it in Kiview, it'll wipe out the user's entire home directory without warning. Tested this in a VM.

Could avoid this in a number of ways, e.g. by not calling bash. I'm not familiar with QProcess usage, so I don't know what the "proper" way to handle command arguments here is, but as a rule of thumb, the file path itself should be treated as a singular argument, so that it can't accidentally be interpreted as a series of arguments that mean something totally different. You could probably also use LibreOfficeKit to avoid making an external command call in the first place.

Attached is a video illustrating a less damaging version:

command-injection.webm

[Nyre221]

Really interesting, I'll see what I can do.

In any case, I don't think it's the call to libreoffice that's the problem.
I also use bash to try to figure out the file type if there is no extension present and that is probably the problem in this case.

I will check every part of the code where I used bash and try to fix it.

For now use the flatpak version.

Thank you very much for pointing out this error to me.

[Nyre221]

@n303p4
Thank you so much once again for reporting the issue.

I eliminated bash -c and gave the arguments directly to the exec and that allowed me to pass the path separately.

In dolphinbridge.cpp I put a check on the presence of some characters so as to avoid using xclip or wl-copy to restore the contents of the clipboard (This is necessary because KIview uses the clipboard to take the path from dolphin).