WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#11154 closed defect (bug) (invalid)

fread() code in pclzip doesn't check the result size

Reported by: dobesv Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords:
Focuses: Cc:

Description

Although this code from class-pclzip.php obviously works for most people, when running inside of teh quercus/PHP interpreter, it fails because quercus returns fewer bytes than requested if more than ~8000 bytes are requested.

        if ($p_entry['compression'] == 0) {
          // ----- Reading the file
          $p_string = @fread($this->zip_fd, $p_entry['compressed_size']);
		echo '<p>Requested '.$p_entry['compressed_size'].' bytes of file '.$p_entry['filename'].' and got '.strlen($p_string)' bytes.</p>';
        }
        else {

          // ----- Reading the file
          $v_data = @fread($this->zip_fd, $p_entry['compressed_size']);

          // ----- Decompress the file
          if (($p_string = @gzinflate($v_data)) === FALSE) {
              // TBC
          }
		  echo '<p>Requested '.$p_entry['compressed_size'].' bytes of file '.$p_entry['filename'].' and got '.strlen($v_data)' bytes.  Expecting uncompressed size '.$p_entry['size'].' and got '.strlen($p_string).' bytes back from gzinflate.</p>';
        }

For example when installing the flutter plugin within wordpress some files are not read in full:

Requested 12787 bytes of file fresh-page/thirdparty/swfupload/swfupload.swf and got 8096
Requested 10020 bytes of file fresh-page/thirdparty/swfupload/swfupload.js and got 8097

It does seem like Quercus is behaving differently than the standard PHP distribution on this one, in that with a normal PHP executable it would be guaranteed to return all the bytes from a local file unless there was an EOF, so I'll also take this up with the folks at Quercus.

My suggestion would be to add these reliable versions of fread and fwrite to file.php (or somewhere similarly suitable) and replace all calls to fread() or fwrite() that *assume* a full write or full read with calls to these functions:

function fullwrite ($sd, $buf) {
  $total = 0;
  $len = strlen($buf);

  while ($total < $len && ($written = fwrite($sd, $buf))) {
    $total += $written;
    $buf = substr($buf, $written);
  }

  return $total;
}

function fullread ($sd, $len) {
  $ret = '';
  $read = 0;

  while ($read < $len && ($buf = fread($sd, $len - $read))) {
    $read += strlen($buf);
    $ret .= $buf;
  }

  return $ret;
}

This would be a bit more defensive in case of unusual environments, since as documented fread() and fwrite() are *not* guaranteed to read or write the full amount requested/provided.

Change History (2)

comment:1 dd324 years ago

This would be a bit more defensive in case of unusual environments, since as documented fread() and fwrite() are *not* guaranteed to read or write the full amount requested/provided.

In the scenario used by PclZip however, The output is specified to follow a certain path. Either, A. Return x bytes, or B. Return x-y bytes in the event that a EOF is hit.

A EOF doesnt worry PclZip, as that's an error condition anyway.. IMO, Your PHP implementation is broken and should be fixed instead..

comment:2 nacin4 years ago

  • Milestone Unassigned deleted
  • Resolution set to invalid
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.