Member Avatar for larry29936

I have a php function which allows users to download files from our archive site in England. The main site is in the U.S. and that function is working correctly. The UK function results in the files having incorrect md5sums. I've gone through it multiple times and can't find any spaces where they shouldn't be and that there's no processing before the header code. I've attached a copy of the code in case someone would like to take a look and maybe spot something I've missed that could be causing this problem.

Thanks in advance.

<?php
function ukdloader($l_filename=NULL){
    if( isset( $l_filename ) ) {
         $file_url='http://foxclone.org/downloads/'.$l_filename;
         header('Content-Type: application/octet-stream');
         header("Content-Disposition: attachment; filename={$l_filename}");
         header('Pragma: no-cache');
         header('Expires: 0');
         readfile($file_url);

         $php_scripts = '../../php/';
         require $php_scripts . 'PDO_Connection_Select.php';
         require $php_scripts . 'GetUserIpAddr.php';

         $ip = GetUserIpAddr();
         if (!$pdo = PDOConnect("foxclone_data"))
         {
           die;
         }

         $ext = pathinfo($l_filename, PATHINFO_EXTENSION);
         $stmt = $pdo->prepare("INSERT INTO download (address, filename,ip_address) VALUES (?, ?, inet_aton('$ip'))");
         $stmt->execute([$ip, $ext]);

         $test = $pdo->query("SELECT lookup.id FROM lookup WHERE inet_aton('$ip') >= lookup.ipstart AND inet_aton('$ip') <= lookup.ipend");
         $ref = $test->fetchColumn();
         $ref = intval($ref);

         $stmt = $pdo->prepare("UPDATE download SET lookup_id = '$ref' WHERE address = '$ip'");
         $stmt->execute();
        }
    }        
    ukdloader($_GET["f"]);
  • 2 Contributors
  • 6 Replies
  • 96 Views
  • 1 Day Discussion Span
  • Latest Post Latest Post by larry29936
Member Avatar for Dani

A few things. Firstly, I’m unsure why you think this is an MD5 issue because I’m not seeing anything in your code that MD5 encodes anything?

Secondly, you never stated specifically what error message you’re getting or what isn’t working. If you are getting a blank page, it’s probably because it can’t connect to the database and so you have that die call there to end the script prematurely.

Thirdly, you’re not escaping your GET input which is a huge security risk. Please sanitize your input so hackers or bad actors can’t do whatever they want to your file system and database.

I’m typing this from my phone in bed right now so my answer is a bit brief. I can go into more detail from my computer later in the day.

Member Avatar for larry29936

@Dani - I should probably start a new topic. I can now download from both the U.S. and UK sites. The problem is that if a user downloads the same file from both sites, they'll have different md5sums. That is a symptom of a coding issue, I believe. I've checked both sets of code and can't find any blank spaces that might cause the problem.

Thirdly, you’re not escaping your GET input which is a huge security risk. Please sanitize your input so hackers or bad actors can’t do whatever they want to your file system and database.

I'm not sure what you're talking about. Escaping my GET input?

Thanks for your help.

Member Avatar for Dani

What does require $php_scripts . 'GetUserIpAddr.php'; do? Does it echo the IP address or the country or anything user specific? That could be a reason different locations are getting seemingly different files. It’s also possible that foxclone.org serves different files based on geolocation. Perhaps there’s a CDN and a file is cached in one geo but not another?

Yes, escape your GET input before using it in a MySQL query with mysql_real_string() and also escape it before using it in the file name to be downloaded.

Member Avatar for larry29936

@Dani - 'require $php_scripts . 'GetUserIpAddr.php';' calls a function to get the user's ip address that is used to create a record in the database.

I still don't understand what you mean by:

Yes, escape your GET input before using it in a MySQL query with mysql_real_string() and also escape it before using it in the file name to be downloaded.

I'm not using the filename in any sql statements.

Member Avatar for Dani

Sorry, I was in bed not feeling well all day yesterday so I wasn't able to properly articulate myself while typing from my phone.

You have the code here:

     $ext = pathinfo($l_filename, PATHINFO_EXTENSION);
     $stmt = $pdo->prepare("INSERT INTO download (address, filename,ip_address) VALUES (?, ?, inet_aton('$ip'))");
     $stmt->execute([$ip, $ext]);

Basically what that code does is look at the extension for whatever filename is passed in, where an extension is simply what comes after a period. Since someone can type anything for l_filename, such as anything.malicous-extension then you don't want to plug that directly into a MySQL query.

Absolutely ANY TIME that any portion of a database query is generated using end-user freeform text, or text derived from end-user freeform text, there's a way of a malicious end-user to exploit that, and so you always want to sanitize.

For the $ext being passed into that MySQL query, you would want to wrap it around this function call: https://www.php.net/manual/en/mysqli.real-escape-string.php

Secondly, you have:

$file_url='http://foxclone.org/downloads/'.$l_filename;
...
readfile($file_url);

The same thing here ... Because $l_filename can be potentially malicious, if $l_filename is something malicious, and then you have PHP code to readfile(), you can potentially be reading and executing malicious code on your server.

What format are you expecting $l_filename to be? If you're expecting it to be just alphanumeric characters, then strip away all non-alphanumeric characters from the string before using it. If you're expecting it to be 25 characters, for example, then prevent end-users from potentially making it 5000 characters before it's attempting to be read by your server.

That's what I mean about sanitizing.

As far as GetUserIpAddr, I was simply coming up with different ideas of what might be causing the discrepancy. Since we don't know exactly what's in that file, it opens it up as a potential issue. Perhaps that file, for example, introduces an accidental blank space in one geo but not another one, for example.

Member Avatar for larry29936

@Dani - thanks for the detailed explanation. I have the filenames in the database. What if I queried the database and just exit the function if the file isn't found? OR just pass the index number (primary key) from the calling script and query for the filename in the function?

Edited by larry29936
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.