Code review comment for lp:~roadmr/ubuntu/oneiric/casper/809885

Hi Colin,

Thanks for your comments!

> To be honest, I think this just highlights the bad way casper was handling
> this file before you touched it (which is my fault, since apparently I
> introduced this code). Rather than patching around the problem, could you
> just make casper read ipconfig's output as shell input using '.
> /tmp/net-"${DEVICE}".conf', and then we can just use the resulting shell
> variables? That would be more reliable long-term than trying to keep up with
> which way ipconfig currently quotes shell variables.

Agreed, looking at ipconfig's history and bug reports it looks like just sourcing the file and using the variables is the recommended way to work with it.

The updated branch does this, I chose not to put the $DNSDOMAIN and IPV4DNS{0,1} directly in the heredoc for clarity, and also because we still need to strip all but the first entry from the data that will go in the "domain" field of resolv.conf. This, by the way, is the only remaining regex in that section of code, and I simplified it a bit. So preparing the variables beforehand and then just outputting them in the heredoc looked like the clearest way to do it.

I hope I didn't make a mess with the branch, my revision 907 should merge cleanly against trunk but the commit history looks a bit messy, let me know if you'd prefer a new branch with just one commit.

> You mention arbitrary code execution in your bug report; but I don't think
> that's a concern here. All this code is running as root, and if ipconfig
> wanted to cause casper to run some evil code then it could just do so itself.
> Furthermore, it's all running in the initramfs, where there is no possibility
> that a malicious user might have injected data into /tmp. The current code in
> casper tries to parse ipconfig's output like this because it's wrong, not
> because it's clever.

Well, this motivation was mentioned in the klibc changelog, I'm not sure what incident prompted it but I agree that for Ubiquity the risk is nearly nonexistent.

Do let me know if this code looks fine or still needs more work.

Thanks again!