0

I decided to look into security more since i am coding everyday and been rooting through websites, tons of information and said the best way is just ask and see what comes back! For my own information and to learn more and more of injections etc. I would like someone to simply let me know if my code samples are injectable? If they are, could you provide how they are and how to solve/fix it?

NOTE: The samples are in my main code structure but put in different ways.

//Sample 1:
$file = '/var/lib/mpd/playlists/'.$_POST['playlistlink'].'.m3u';
shell_exec('/sbin/sudo /sbin/chmod 664 "'.$file.'"'); //Perhaps? file.mp3"; ls; echo "success

//Sample 2:
$file = '/mnt/MPD/SpotifyPlaylists/'.$_GET['track'].''; //Possible to manipulate to remove more files?
if (file_exists($file) && unlink($file))
{
    echo "Success!";
}
William
  • 1,009
  • 15
  • 41
  • yes of course `$file = '/var/lib/mpd/playlists/'.$_POST['playlistlink'].'.m3u';`. So long as you are directly posting without any form of sanitization, it is vulnerable. A malicious user can easily do funny things like for example: `$file = '/var/lib/mpd/playlists/file://.m3u';`. Attacks such as XSS, Directory transversal attempts are swimming around your code – Rotimi May 15 '17 at 10:07
  • I had a strong feeling that was! Is my perhaps correct or is there a different way of doing it? How could i patch this up to prevent it? – William May 15 '17 at 10:09
  • You should filter the request value for XSS. Please check it http://stackoverflow.com/questions/1336776/xss-filtering-function-in-php – StreetCoder May 15 '17 at 10:09
  • This question type fits more on https://codereview.stackexchange.com/ – k0pernikus May 15 '17 at 10:11
  • Would `$file = strip_tags($file);` Be efficient? @StreetCoder I was literally just looking at that before the post. – William May 15 '17 at 10:16
  • 1
    @irishwill200 Always treat user-generated input as harmful unless proven otherwise. Never use POST data without sanitizing it first. Avoid code execution like `shell_exec`, esp. since you just want to [chmod](http://php.net/manual/de/function.chmod.php). Ask yourself: Do I need it? You also are calling `sudo` which means somehow that scripts runs as root? Why? Never let your scripts run as root or the can wreck even more havok on successful code injection. Set up your server in way that no scripts needs root access but make do with group access to certain folders. – k0pernikus May 15 '17 at 10:16
  • 1
    Last but not least: Don't re-invent the wheel. Look up frameworks like [symfony3](http://symfony.com/doc/current/setup.html) That comes with some reasonable defaults. – k0pernikus May 15 '17 at 10:21
  • @k0pernikus Thanks very much for the information! I take all on board and will check over the code to see what can be changed and made more secure! Thanks for your output! – William May 15 '17 at 10:25
  • @k0pernikus Forgot to mention, the reason for shell_exec and sudo etc is because the user is not the owner of the file so has to sudo change it. – William May 15 '17 at 10:38
  • @irishwill200 Your `sudo` is an attempted solution to the file permission problem. It has huge security downsides and you don't need it, there are other ways. You should only allow your scripts to read, write and delete files on certain folders. (That's what I mean by setting up group access.) – k0pernikus May 15 '17 at 10:46

0 Answers0