Project

General

Profile

Bug #5481

HTML Status Codes not Processed Properly from download_file

Added by Daniel Subert about 4 years ago. Updated over 3 years ago.

Status:
Duplicate
Priority:
Normal
Category:
-
Target version:
-
Start date:
11/18/2015
Due date:
% Done:

0%

Estimated time:
Affected Version:
Affected Architecture:

Description

The download_file function returns status codes as true if 200, or the status code if another value. However, this is then used in if functions poorly and sometimes not checked at all before calling other functions relying on the file downloaded. The problem is that status codes are not 0 so an if(download_file(...)) will always evaluate true, even for a 404 not found or 401 forbidden.
http://php.net/manual/en/language.types.boolean.php

download_file should only return the status code, and should always be checked for the code to be successful according to http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

The function download_file also does not handle a curl error, so if a status 200 download fails in the middle of downloading the file, the file can still be processed.
http://php.net/manual/en/function.curl-errno.php

If would suggest returning http errors as positive numbers and curl errors as negative numbers to differentiate the type of error and processing the errors properly in every location this function is used. Then you can use
$result = download_file(...);
if($result matches as success code) {}
else { process $result error code }

This issue is marked Very High as the firmware and package download uses this function to update pfSense.

History

#1 Updated by Chris Buechler about 4 years ago

  • Status changed from New to Confirmed
  • Priority changed from Very High to Normal

it's no longer used for package or update downloads in 2.3. The only thing that uses it in 2.3 is alias downloads. Those should have better validation though.

#2 Updated by Jim Thompson over 3 years ago

  • Assignee set to Renato Botelho

#3 Updated by Chris Buechler over 3 years ago

  • Status changed from Confirmed to Duplicate
  • Target version deleted (Future)
  • Affected Version deleted (All)

I fixed this in #5848

Also available in: Atom PDF