Help with PHP script to download MP3 files

[thisisgraham]

Hey everyone,

 

I am looking after a website at the moment (and I have no real PHP experience) and there's an issue where users are unable to download MP3 files that have been uploaded. This is a hosted WordPress site using a custom theme.

 

The issue presents itself here: http://www.everydaychurch.com/resources/sermons/ 

 

If you click on the 'Download' links it will download an empty .mp3 file (with the correct file name).

 

The PHP script it uses is:

 

<?php
header('Content-disposition: attachment; filename='. urlencode($_GET["fname"]). '.mp3');
header('Content-type: audio/mpeg');




$path_parts = pathinfo($_GET["file"]);


if ($path_parts['extension'] == "mp3"){
readfile($_GET["file"]);
}?> 

If you check the URL of the download links it does actually contain the correct file location for the MP3, but clicking the link (for whatever reason) doesn't work. I would really appreciate any help or insight into this issue! 

 

Cheers,

Graham

[requinix]

Sample for those who don't want to browse:

http://www.everydaychurch.com/wp-content/themes/Malachi_slider_update/includes/mp3.php?file=http://www.everydaychurch.com/wp-content/uploads/2017/04/Consumed-By-Generosity-Mark-Perry-April-2-2017.mp3&fname=CONSUMED%20BY%20GENEROSITY

Since the file path there is correct and the "extension" will be mp3, it suggests the server is not able to retrieve the file from... itself. Could be a server configuration issue.

 

But really, using the whole URL there is not good - it's not just unsafe but it puts additional strain on the server. You should be using a path like

?file=2017/04/Consumed-By-Generosity-Mark-Perry-April-2-2017.mp3

You can then figure out where on the server the file is and read the file itself directly from the disk. Much more efficient this way. And it will definitely work.

 

Plus, you need to be careful about letting people download things. Very, very careful. Make sure the file they want to get is something you also want them to get.

Checking the extension is good but having more checks in there would be better.

<?php

// validate the file before doing anything
$file = $_GET["file"];
// regular expressions are nice; file is YYYY/MM/*.mp3
if (!preg_match('#^\d\d\d\d/\d\d/[^/.]+\.mp3$#', $file)) {
	// an error message would be nice, but it's not required
	exit; // don't do anything else
}

// $file is located in /wp-content/uploads
$file = $_SERVER["DOCUMENT_ROOT"] . "/wp-content/uploads/" . $file; // full path

// now you have to check that it actually exists
if (!is_file($file)) {
	// again, an error message would be nice
	exit; // again, don't do anything else
}

// now we're sure that $file is safe to allow for downloading

header('Content-disposition: attachment; filename='. urlencode($_GET["fname"]). '.mp3');
header('Content-length: ' . filesize($file)); // the browser needs to know the total size to show download progress
header('Content-type: audio/mpeg');

readfile($file);
?>

[thisisgraham]

Hey,

 

Thanks for the quick reply! Unfortunately a lot of what you've said is over my head - presumably the actual button itself needs to be changed as well as the PHP script to implement your suggestions? 

 

Is there an actual problem (other than security) with the way the script is currently configured, or is at (as you said) an issue with the server configuration disallowing the file to be downloaded? 

 

Thanks again for your help!

 

G

[requinix]

Yes, there is an actual problem with the script: it's downloading from the file from your own server (bad). It's surprising that doesn't work, but point is it should not be doing that in the first place.

 

Are you not able to change the button? There is an alternative if you really can't do that, but it's not that great so fixing the button would be better.

[Jacques1]

The downloaded files aren't empty. They say this:

<br />
<b>Warning</b>:  readfile(): http:// wrapper is disabled in the server configuration by allow_url_fopen=0 in <b>/home/content/20/9536120/html/wp-content/themes/Malachi_slider_update/includes/mp3.php</b> on line <b>10</b><br />
<br />
<b>Warning</b>:  readfile(http://www.everydaychurch.com/wp-content/uploads/2017/04/Consumed-By-Generosity-Mark-Perry-April-2-2017.mp3): failed to open stream: no suitable wrapper could be found in <b>/home/content/20/9536120/html/wp-content/themes/Malachi_slider_update/includes/mp3.php</b> on line <b>10</b><br />

.

 

 

 

Let's just say it's divine intervention to prevent you from causing harm to the site users or the server.

 

Really, WTF are you doing? I understand that you're new to PHP, but then you either have to learn it or give up. Compromising security for a bunch of MP3 files is insane.

 

What is the script supposed to do, anyway? If all you want is a forced download, use the download attribute:

<a href="/path/to/file.mp3" download="name_for_saved_file.mp3">Click to download</a>

This is supported by all modern browsers except for Internet Explorer.

 

IE will either download the file as well or open it in a player. The former would be great, the latter shouldn't be that much of an issue, because the user can always save the file within the player.