On 11/11/14 13:05, Cynt Rynt wrote: > Hi, > Ran scan-build on toybox and found some errors. This fix removed an > error in password.c, but I'm not sure if it's the right fix. > Thanks, > Cindy
Thanks, let's take a look... I suspect the correct fix is just not to save the return code. It's a speculative unlink: the file may not be there to be unlinked (most likely it won't be, it's just a backup), and if it is still there the link() should fail and we process the error from that. Now it's possible that you could have a symlink you can't unlink, and thus the link might move the file to the wrong location (although it won't cross filesystems and the result would still have the original permissions), but if someone can create a specific file in /etc that _root_ can't remove, the system is already compromised. This function has a larger problem that it assumes colon separated /etc/passwd format and android doesn't use that. (Android decided that the windows system registry was a good idea, and made a big global database in a binary format for critical system information to live in, because reasons.) That's why I haven't done a cleanup pass on this function yet: I don't know what to do about the design issue. I need to set up an AOSP build environment, and my obvious machine to do that on is stuck on Ubuntu 13.04 because the build servers went down and it won't upgrade anymore, so I need to back it up and reinstall it... Thanks for the heads up, I checked in the quick one line fix. I'll have to take a proper look at this function after the 0.5.1 release on saturday. Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
