[SUCS Devel] [Git][sucssite/sucs-site][sucs-site] 3 commits: stop .forward file being a symlink

[Andrew Price]

On 02/04/17 21:17, Imran Hussain wrote:
> On 02.04.2017 06:49, Andrew Price wrote:
>> On 01/04/17 22:05, Imran Hussain wrote:
>>> On 01.04.2017 19:33, Andrew Price wrote:
>>>> - What if ~/.forward is replaced by a symlink between the is_link()
>>>> call and the file() call?
>>>
>>> What a attack!
>>
>> https://cwe.mitre.org/data/definitions/363.html
>>
>> It's pretty straightforward, something brutish like
>>
>>   while (1) {
>>           rename(".forward", ".forward.backup");
>>           symlink("somefile", ".forward");
>>           rename(".forward.backup", ".forward");
>>   }
>>
>> would probably get "somefile" read about 10% of the time. Opening
>> with O_NOFOLLOW and then doing all subsequent operations on the file
>> descriptor is a good way to fix it but I doubt there's an easy way to
>> get PHP to use that flag.
>
> Could just read the file and do input validation? .forward is supposed
> to contain a email address, so why not just read and then use php email
> validation stuff to check. OR have code in the sucssite that reads the
> first line of a filer and looks for a string and if that's there then
> don't read the rest of the file? So in cases of the original exploit
> have "//pls don't display on the sucs site" as the first line and get
> the sites code to check for that?

You'd still be able to read someone else's .forward in that case?

One way to do it would be to do something like (pseudocode):

   fd = open("/home/foo/.forward");
   readlink("/proc/self/fd/" . $fd, &link);
   if (link == "/home/foo/.forward") {
         // read it
   }
   close(fd);

Andy