On 07/01/16 19:41, anonym wrote:
>
> I haven't tested it, but I did have a look at src/tcp-helper.c since the
> necessity of memory unsafe code surprised me:
For the record, I never intended to use C. It was originally written in
perl (like the GUI wrapper), but setuidperl is no longer supported and I
needed to drop any requirement for sudo. The perl documentation suggests
writing a setuid C wrapper that does (more or less)
"setreuid(0,0); system(MY_PROGRAM);"
and then lists all the reasons why you shouldn't.
So I reluctantly rewrote the helper in C, and after getting angry with C
I gave in and #included <string> (forgetting to rename the source
accordingly). Much of my time since has been spent trying to extricate
myself from the inevitable foot-shooting. ;-)
Rest of your comments fully understood, hands in the air bang to rights guv.
> Best practice is to drop the input sanitation and pick alternatives to
> `system()`/`popen()` that avoids invoking the shell and calls `exec()`
> or similar. I'm too ignorant of C/C++ to recommend you anything.
Yes, these were originally perl system(,,,) and open(,,,) calls, which
are safe as they don't invoke a shell. C has no simple equivalent that I
am aware of, so I cheated by adding the overkill input sanitation
(bearing in mind that in normal use the input parameters will in 99.9%
of cases be "/live/persistence/TailsData_unlocked" and "/dev/sd[a-z]").
> So, I personally would not include this program. Hence I recommend you
> to rewriting it in the same scripting language you end up using for the
> GUI, so Python 3, it seems.
I'd pay good money to have setuidperl back, but upstream had their reasons.
Thanks for the thorough criticism, sincerely. So I have a few questions
for the list:
1. is it worth spending any more time fixing the dodgy C++, or is it
just throwing good effort after bad?
2. is the general structure of userspace GUI + setuid helper still the
way to go?
3. Since tails-installer-invoker and tails-installer remain distinct
programs (and tails-installer closely tracks liveusb-creator), is it
necessary/desirable to merge persistence cloning into either of them, or
should/could it remain a separate (linked) executable? In other words,
do I really need to learn Python? ;-)
A
