-1

Can any one exploit my below code :-

    $myfile = fopen("chat.txt", "a") or die("Unable to save!");
    $content = trim($_POST['message'])."\n";
    fwrite($myfile, $content);
    fclose($myfile);

I am making small chat application where, i am writing content in text file rather than mysql. There is not much performance change, but i am preferring file as it is little faster than mysql.

My only concern is , "Can any one exploit" above code ?

I don't see any way, but Just asking, If i am missing anything.

John Cargo
  • 1,839
  • 2
  • 29
  • 59
  • http://stackoverflow.com/questions/1952875/security-vulnerabilities-in-php-fwrite Take a look at the answers on that – Matt Jan 16 '16 at 23:19
  • as a txt file its ok but if someone call find a way to call it from your server, i.e. `index.php?page=chat.txt` & say you are including files based on url variables ($_GET['page']), then you can insert php objects. – David Jan 16 '16 at 23:44
  • @Theraot This wouldn't fit Code Review. It's a specific question, Code Review can target any and all parts of the code. – Mast Jan 17 '16 at 01:59
  • @Mast I have seen similar questions there, on the format of "I have done this code that does X and Y, it works, but I wonder if it is safe. Will people be able to exploit it? [code listing]" – Theraot Jan 17 '16 at 02:04
  • @Theraot [A guide to Code Review for Stack Overflow users](http://meta.codereview.stackexchange.com/questions/5777/a-guide-to-code-review-for-stack-overflow-users/5778#5778) – Mast Jan 17 '16 at 02:09

2 Answers2

2

Yes, they are called XSS attacks. They will get you not on how you store the data, but rather when you print it out.

Basically the attacks can include javascript code that will run on the clients computer to steal their cookies like for example the session ID wich is of the attackers interest.

You can avoid this by encoding it using htmlentities() and decode it with html_entity_decode().

Marco
  • 7,007
  • 2
  • 19
  • 49
Xorifelse
  • 7,878
  • 1
  • 27
  • 38
  • Well, it's a good reminder for the code actually outputting the log. But is it really relevant enough to be an answer? (technically not a problem with the above code) – JimL Jan 16 '16 at 23:34
  • Other than that I see no issues, other then just think the step ahead. Like just for logging it is alright but maybe you want to print the chat line by line on your page and there is the security risk. – Xorifelse Jan 16 '16 at 23:38
  • 1
    Imho that would qualify it as a useful comment, not an answer – JimL Jan 16 '16 at 23:39
2
  • message can contain \n, so I could break order/flow if you depend on that,
  • message length is not limited I could make you run out of space,
  • there is no limit on message number, your IO is mine,
  • message could contain XSS (like @Xorifelse mentioned),
  • message could contain control character, that could be annoying e.g. with cat
kwarunek
  • 12,141
  • 4
  • 43
  • 48
  • It should be noted that there is an implicit size limit to the post enforced by any decent web server - which could be malconfigured. (+1) – Theraot Jan 17 '16 at 01:57