Skip to content

Commit

Permalink
Disallow wb mode in favor of w+b mode.
Browse files Browse the repository at this point in the history
  • Loading branch information
paragonie-security committed Jan 25, 2018
1 parent a72ec65 commit de63ad1
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
8 changes: 5 additions & 3 deletions src/Stream/MutableFile.php
Expand Up @@ -28,7 +28,7 @@
*/
class MutableFile implements StreamInterface
{
const ALLOWED_MODES = ['r+b', 'wb', 'w+b', 'cb', 'c+b'];
const ALLOWED_MODES = ['r+b', 'w+b', 'cb', 'c+b'];
const CHUNK = 8192; // PHP's fread() buffer is set to 8192 by default

/**
Expand Down Expand Up @@ -70,7 +70,7 @@ public function __construct($file)
'Could not open file for writing'
);
}
$fp = \fopen($file, 'wb');
$fp = \fopen($file, 'w+b');
// @codeCoverageIgnoreStart
if (!\is_resource($fp)) {
throw new FileAccessDenied(
Expand Down Expand Up @@ -166,8 +166,10 @@ public function readBytes(int $num, bool $skipTests = false): string
if ($remaining <= 0) {
break;
}
/** @var int $bufSize */
$bufSize = \min($remaining, self::CHUNK);
/** @var string $read */
$read = \fread($this->fp, $remaining);
$read = \fread($this->fp, $bufSize);
if (!\is_string($read)) {
throw new FileAccessDenied(
'Could not read from the file'
Expand Down
28 changes: 26 additions & 2 deletions test/unit/StreamTest.php
Expand Up @@ -32,6 +32,8 @@ public function testFileHash()
$fileOne->getHash(),
$fileTwo->getHash()
);
$this->assertSame(65537, $fileOne->getSize());

fclose($fp);
}

Expand Down Expand Up @@ -94,14 +96,14 @@ public function testResource()
);
}

$writable = \fopen($filename, 'wb');
$writable = \fopen($filename, 'w+b');
$wstream = new MutableFile($writable);
$this->assertInstanceOf(MutableFile::class, $wstream);
try {
new ReadOnlyFile($writable);
} catch (CryptoException\FileAccessDenied $ex) {
$this->assertSame(
'Resource is in wb mode, which is not allowed.',
'Resource is in w+b mode, which is not allowed.',
$ex->getMessage()
);
}
Expand Down Expand Up @@ -141,6 +143,17 @@ public function testFileRead()
$buf
);
$fStream->reset(0);

try {
$fStream->readBytes(-1);
$this->fail('Allowed to read -1 bytes');
} catch (CryptoException\CannotPerformOperation $ex) {
}
try {
$fStream->readBytes(65538);
$this->fail('Allowed to read more bytes than the file contains');
} catch (CryptoException\CannotPerformOperation $ex) {
}

file_put_contents(
$filename,
Expand All @@ -155,5 +168,16 @@ public function testFileRead()
$ex instanceof CryptoException\FileModified
);
}
foreach ([255, 65537] as $size) {
$buffer = random_bytes($size);
$fileWrite = tempnam('/tmp', 'x');
$mStream = new MutableFile($fileWrite);
$mStream->writeBytes($buffer);
$mStream->reset(0);

$this->assertSame(0, $mStream->getPos());
$this->assertSame($size, $mStream->getSize());
$this->assertSame(bin2hex($buffer), bin2hex($mStream->readBytes($size)));
}
}
}

0 comments on commit de63ad1

Please sign in to comment.