search a php string for two substrings

Question

I am programming a php program that will allow users to download audio from my website.
To do this they go to www.mysite.com/downloadit.php?file=myfile.mp3 and the download of myfile.mp3 will begin immediately.

There is a problem though: I don't want people to be able to download system files. I am going to solve this by checking if $_GET['file'] contains the substrings .mp3 or .wav. I am trying to do this with the strpos command, but can't get it working. How would I check for more than one substring (.mp3 and .wav) in a string with strpos? Or maybe I should use a different command? Please let me know!
Here is my code so far:

$haystack=$_GET['file'];

$resulting = strpos($haystack, ".mp3");

//if found $resulting will equal 1
//if not found $resulting will equal 0

//if $resulting is less than one then it must be 0

    if($resulting < 1){
    //security breach!
    die("Unauthorized");
}

//assuming we passed the last test, everything went well, we will then continue

    else{
    //code here
}

Thanks to @DoubleSharp i now have this completed code!!!

//if it is found $resulting will not equal 0
//if it is not found $resulting will equal 0

//echo the result
//echo $resulting;

//add a line break
echo "<br/>";
//if $resulting is less than one then it must be 0
//since it is 0 it must mean that it is a breach!
if (preg_match("~\.(mp3|wav)$~i", $haystack))
{
  echo "hi nice file";
}
else
{
  die("bad file");
}
?>

substr_count() and there you goooo. Though I have high doubts about the security ... — HamZa
– HamZa, Commented May 15, 2013 at 21:25
For better security, consider maintaining a list of files that may be downloaded and comparing the input to that list. At the very least, limit accessible files to a particular directory or set of directories. — George Cummins
– George Cummins, Commented May 15, 2013 at 21:26
People are uploading audio onto my website all the time, mantaing that list would require a whole new set of code. This way would be much easier. This is just a backup security system really.... I have one in front of this that makes sure they don't download outside of the /audio/ directory. Thx though — Shadowpat
– Shadowpat, Commented May 15, 2013 at 21:28
Put your downloadable files under a single directory. When you get a filename from the client, check for the existence of the file in that directory only (filtering '.' and '..'). Otherwise, security is going to be an issue. Make sure you don't allow for any kind of path in the filename.... — Madbreaks
– Madbreaks, Commented May 15, 2013 at 21:29
@Shadowpat This isn't a great strategy ... You will have name collisions. Save the names into a DB, save the files in your folder with a unique hash as name. — HamZa
– HamZa, Commented May 15, 2013 at 21:37

doublesharp · Accepted Answer · 2013-05-15 21:39:40Z

2

You can use regular expressions to test for multiple values, specifically preg_match(). If you use the pattern \.(mp3|wav)$~i wrapped in a delimiter (~ in this case) it will match strings that end with a literal dot . followed by either mp3 or wav. The $ in the pattern matches the end of the line, and the i modifier at the end tells it to do case insensitive matching, so file.MP3 and file.mp3 will both match.

if ( preg_match("~\.(mp3|wav)$~i", $haystack) ) {
    // it matches
}

edited May 15, 2013 at 21:39

answered May 15, 2013 at 21:24

doublesharp

27.9k6 gold badges56 silver badges76 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Shadowpat Over a year ago

What is the difference with the ~'s and without?

Madbreaks Over a year ago

@Shadowpat They're simply RegEx delimiters. Use what you like, I tend to use /

doublesharp Over a year ago

@Shadowpat You can use any matching characters in place of the ~, it's just a delimiter. A forward slash is somewhat standard, but can get confusing when matching URLs since you have to escape them, which is why I opt for a tilde in these cases.

doublesharp Over a year ago

It also lets you add modifiers after your pattern - in this case i to make it case insensitive.

Shadowpat Over a year ago

@doublesharp I keep getting an error (new code and error posted above)

|

Madbreaks · Accepted Answer · 2013-05-15 21:34:01Z

2

I'd suggest something like:

$allowed = array('mp3', 'wav', 'ogg'); //whatever

$file = basename($_GET['file']);  // strip the path! (if any)

if(!preg_match("/\.(?:".implode('|', $allowed).")$/", $file){
   // Show 404, or whatever, exit
}

// Now check under trusted directories for the file,
// which should help to ensure that no system files are
// accessed since there shouldn't be any in there

answered May 15, 2013 at 21:34

Madbreaks

19.6k7 gold badges62 silver badges75 bronze badges

3 Comments

Shadowpat Over a year ago

@doublesharp has the better code, thank you though.. upvote! Also, I will use the idea for the allowed directories

Madbreaks Over a year ago

Thanks - I prefer having things like allowed file types kept separately from (for example) a RegEx which checks for them, hence the dynamic RegEx here. Also thought the basename idea was a good one.

HamZa Over a year ago

This is a better solution since it's stripping the path +1

Collectives™ on Stack Overflow

search a php string for two substrings

2 Answers 2

7 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related