1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-19 13:22:42 +01:00

Use "\z" instead of "$" to anchor validating regular expressions

Summary:
Via HackerOne. In regular expressions, "$" matches "end of input, or before terminating newline". This means that the expression `/^A$/` matches two strings: `"A"`, and `"A\n"`.

When we care about this, use `\z` instead, which matches "end of input" only.

This allowed registration of `"username\n"` and similar.

Test Plan:
  - Grepped codebase for all calls to `preg_match()` / `preg_match_all()`.
  - Fixed the ones where this seemed like it could have an impact.
  - Added and executed unit tests.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: aran, epriestley

Differential Revision: https://secure.phabricator.com/D8516
This commit is contained in:
epriestley 2014-03-13 12:42:41 -07:00
parent 58877a9b6c
commit 969d0c3e8d
15 changed files with 27 additions and 13 deletions

View file

@ -114,7 +114,7 @@ final class PhabricatorAuthProviderOAuth1JIRA
if (!strlen($values[$key_name])) { if (!strlen($values[$key_name])) {
$errors[] = pht('JIRA instance name is required.'); $errors[] = pht('JIRA instance name is required.');
$issues[$key_name] = pht('Required'); $issues[$key_name] = pht('Required');
} else if (!preg_match('/^[a-z0-9.]+$/', $values[$key_name])) { } else if (!preg_match('/^[a-z0-9.]+\z/', $values[$key_name])) {
$errors[] = pht( $errors[] = pht(
'JIRA instance name must contain only lowercase letters, digits, and '. 'JIRA instance name must contain only lowercase letters, digits, and '.
'period.'); 'period.');

View file

@ -41,7 +41,7 @@ final class ConduitAPI_diffusion_getcommits_Method
$commits = array_fill_keys($commits, array()); $commits = array_fill_keys($commits, array());
foreach ($commits as $name => $info) { foreach ($commits as $name => $info) {
$matches = null; $matches = null;
if (!preg_match('/^r([A-Z]+)([0-9a-f]+)$/', $name, $matches)) { if (!preg_match('/^r([A-Z]+)([0-9a-f]+)\z/', $name, $matches)) {
$results[$name] = array( $results[$name] = array(
'error' => 'ERR-UNPARSEABLE', 'error' => 'ERR-UNPARSEABLE',
); );

View file

@ -365,7 +365,7 @@ final class DiffusionRepositoryCreateController
$c_call->setError(pht('Required')); $c_call->setError(pht('Required'));
$page->addPageError( $page->addPageError(
pht('You must choose a callsign for this repository.')); pht('You must choose a callsign for this repository.'));
} else if (!preg_match('/^[A-Z]+$/', $v_call)) { } else if (!preg_match('/^[A-Z]+\z/', $v_call)) {
$c_call->setError(pht('Invalid')); $c_call->setError(pht('Invalid'));
$page->addPageError( $page->addPageError(
pht('The callsign must contain only UPPERCASE letters.')); pht('The callsign must contain only UPPERCASE letters.'));

View file

@ -233,7 +233,7 @@ final class DiffusionSSHSubversionServeWorkflow
rtrim($repository->getLocalPath(), '/'), rtrim($repository->getLocalPath(), '/'),
$path); $path);
if (preg_match('(^/diffusion/[A-Z]+/$)', $path)) { if (preg_match('(^/diffusion/[A-Z]+/\z)', $path)) {
$path = rtrim($path, '/'); $path = rtrim($path, '/');
} }

View file

@ -54,7 +54,7 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
protected function loadRepository($path) { protected function loadRepository($path) {
$viewer = $this->getUser(); $viewer = $this->getUser();
$regex = '@^/?diffusion/(?P<callsign>[A-Z]+)(?:/|$)@'; $regex = '@^/?diffusion/(?P<callsign>[A-Z]+)(?:/|\z)@';
$matches = null; $matches = null;
if (!preg_match($regex, $path, $matches)) { if (!preg_match($regex, $path, $matches)) {
throw new Exception( throw new Exception(

View file

@ -43,7 +43,7 @@ final class DivinerAtomRef {
public function setName($name) { public function setName($name) {
$normal_name = self::normalizeString($name); $normal_name = self::normalizeString($name);
if (preg_match('/^@[0-9]+$/', $normal_name)) { if (preg_match('/^@[0-9]+\z/', $normal_name)) {
throw new Exception( throw new Exception(
"Atom names must not be in the form '/@\d+/'. This pattern is ". "Atom names must not be in the form '/@\d+/'. This pattern is ".
"reserved for disambiguating atoms with similar names."); "reserved for disambiguating atoms with similar names.");

View file

@ -52,7 +52,7 @@ abstract class DivinerWorkflow extends PhabricatorManagementWorkflow {
} }
$book['root'] = Filesystem::resolvePath($book['root'], $full_path); $book['root'] = Filesystem::resolvePath($book['root'], $full_path);
if (!preg_match('/^[a-z][a-z-]*$/', $book['name'])) { if (!preg_match('/^[a-z][a-z-]*\z/', $book['name'])) {
$name = $book['name']; $name = $book['name'];
throw new PhutilArgumentUsageException( throw new PhutilArgumentUsageException(
"Book configuration '{$book_path}' has name '{$name}', but book names ". "Book configuration '{$book_path}' has name '{$name}', but book names ".

View file

@ -113,7 +113,7 @@ final class PhabricatorLocalDiskFileStorageEngine
// Make sure there's no funny business going on here. Users normally have // Make sure there's no funny business going on here. Users normally have
// no ability to affect the content of handles, but double-check that // no ability to affect the content of handles, but double-check that
// we're only accessing local storage just in case. // we're only accessing local storage just in case.
if (!preg_match('@^[a-f0-9]{2}/[a-f0-9]{2}/[a-f0-9]{28}$@', $handle)) { if (!preg_match('@^[a-f0-9]{2}/[a-f0-9]{2}/[a-f0-9]{28}\z@', $handle)) {
throw new Exception( throw new Exception(
"Local disk filesystem handle '{$handle}' is malformed!"); "Local disk filesystem handle '{$handle}' is malformed!");
} }

View file

@ -48,7 +48,7 @@ final class PhabricatorMacroEditController
if (!strlen($macro->getName())) { if (!strlen($macro->getName())) {
$errors[] = pht('Macro name is required.'); $errors[] = pht('Macro name is required.');
$e_name = pht('Required'); $e_name = pht('Required');
} else if (!preg_match('/^[a-z0-9:_-]{3,}$/', $macro->getName())) { } else if (!preg_match('/^[a-z0-9:_-]{3,}\z/', $macro->getName())) {
$errors[] = pht( $errors[] = pht(
'Macro must be at least three characters long and contain only '. 'Macro must be at least three characters long and contain only '.
'lowercase letters, digits, hyphens, colons and underscores.'); 'lowercase letters, digits, hyphens, colons and underscores.');

View file

@ -610,7 +610,7 @@ EOBODY;
return false; return false;
} }
return (bool)preg_match('/^[a-zA-Z0-9._-]*[a-zA-Z0-9_-]$/', $username); return (bool)preg_match('/^[a-zA-Z0-9._-]*[a-zA-Z0-9_-]\z/', $username);
} }
public static function getDefaultProfileImageURI() { public static function getDefaultProfileImageURI() {

View file

@ -49,7 +49,7 @@ final class PhabricatorUserEmail extends PhabricatorUserDAO {
// To this end, we're roughly verifying that there's some normal text, an // To this end, we're roughly verifying that there's some normal text, an
// "@" symbol, and then some more normal text. // "@" symbol, and then some more normal text.
$email_regex = '(^[a-z0-9_+.!-]+@[a-z0-9_+:.-]+$)i'; $email_regex = '(^[a-z0-9_+.!-]+@[a-z0-9_+:.-]+\z)i';
if (!preg_match($email_regex, $address)) { if (!preg_match($email_regex, $address)) {
return false; return false;
} }

View file

@ -27,6 +27,13 @@ final class PhabricatorUserEmailTestCase extends PhabricatorTestCase {
'@@' => false, '@@' => false,
'@' => false, '@' => false,
'user@' => false, 'user@' => false,
"user@domain.com\n" => false,
"user@\ndomain.com" => false,
"\nuser@domain.com" => false,
"user@domain.com\r" => false,
"user@\rdomain.com" => false,
"\ruser@domain.com" => false,
); );
foreach ($tests as $input => $expect) { foreach ($tests as $input => $expect) {

View file

@ -36,6 +36,13 @@ final class PhabricatorUserTestCase extends PhabricatorTestCase {
'a,lincoln' => false, 'a,lincoln' => false,
'a&lincoln' => false, 'a&lincoln' => false,
'a/lincoln' => false, 'a/lincoln' => false,
"username\n" => false,
"user\nname" => false,
"\nusername" => false,
"username\r" => false,
"user\rname" => false,
"\rusername" => false,
); );
foreach ($map as $name => $expect) { foreach ($map as $name => $expect) {

View file

@ -48,7 +48,7 @@ final class PhluxEditController extends PhluxController {
if (!strlen($key)) { if (!strlen($key)) {
$errors[] = pht('Variable key is required.'); $errors[] = pht('Variable key is required.');
$e_key = pht('Required'); $e_key = pht('Required');
} else if (!preg_match('/^[a-z0-9.-]+$/', $key)) { } else if (!preg_match('/^[a-z0-9.-]+\z/', $key)) {
$errors[] = pht( $errors[] = pht(
'Variable key "%s" must contain only lowercase letters, digits, '. 'Variable key "%s" must contain only lowercase letters, digits, '.
'period, and hyphen.', 'period, and hyphen.',

View file

@ -79,7 +79,7 @@ final class ConduitAPI_repository_create_Method
$repository->setName($request->getValue('name')); $repository->setName($request->getValue('name'));
$callsign = $request->getValue('callsign'); $callsign = $request->getValue('callsign');
if (!preg_match('/^[A-Z]+$/', $callsign)) { if (!preg_match('/^[A-Z]+\z/', $callsign)) {
throw new ConduitException('ERR-BAD-CALLSIGN'); throw new ConduitException('ERR-BAD-CALLSIGN');
} }
$repository->setCallsign($callsign); $repository->setCallsign($callsign);