From 1bef1b961dbf59af780cf47b837f9d67d6978460 Mon Sep 17 00:00:00 2001 From: Klaus Silveira Date: Sun, 29 Jun 2014 23:54:03 -0300 Subject: [PATCH] Added proper input escaping --- src/GitList/Application.php | 4 ++++ src/GitList/Controller/BlobController.php | 2 ++ src/GitList/Controller/CommitController.php | 3 +++ src/GitList/Controller/MainController.php | 2 ++ src/GitList/Controller/NetworkController.php | 8 +++++--- src/GitList/Controller/TreeController.php | 4 ++++ src/GitList/Escaper/ArgumentEscaper.php | 15 +++++++++++++++ 7 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 src/GitList/Escaper/ArgumentEscaper.php diff --git a/src/GitList/Application.php b/src/GitList/Application.php index 10787e2..18cc681 100644 --- a/src/GitList/Application.php +++ b/src/GitList/Application.php @@ -69,6 +69,10 @@ class Application extends SilexApplication return $twig; })); + $this['escaper.argument'] = $this->share(function() { + return new Escaper\ArgumentEscaper(); + }); + // Handle errors $this->error(function (\Exception $e, $code) use ($app) { if ($app['debug']) { diff --git a/src/GitList/Controller/BlobController.php b/src/GitList/Controller/BlobController.php index fc5ec90..86cef8b 100644 --- a/src/GitList/Controller/BlobController.php +++ b/src/GitList/Controller/BlobController.php @@ -43,6 +43,7 @@ class BlobController implements ControllerProviderInterface )); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', '.+') + ->convert('commitishPath', 'escaper.argument:escape') ->bind('blob'); $route->get('{repo}/raw/{commitishPath}', function ($repo, $commitishPath) use ($app) { @@ -66,6 +67,7 @@ class BlobController implements ControllerProviderInterface return new Response($blob, 200, $headers); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', $app['util.routing']->getCommitishPathRegex()) + ->convert('commitishPath', 'escaper.argument:escape') ->bind('blob_raw'); return $route; diff --git a/src/GitList/Controller/CommitController.php b/src/GitList/Controller/CommitController.php index 325ca71..292e94c 100644 --- a/src/GitList/Controller/CommitController.php +++ b/src/GitList/Controller/CommitController.php @@ -61,6 +61,7 @@ class CommitController implements ControllerProviderInterface })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', $app['util.routing']->getCommitishPathRegex()) ->value('commitishPath', null) + ->convert('commitishPath', 'escaper.argument:escape') ->bind('commits'); $route->post('{repo}/commits/{branch}/search', function (Request $request, $repo, $branch = '') use ($app) { @@ -89,6 +90,7 @@ class CommitController implements ControllerProviderInterface )); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('branch', $app['util.routing']->getBranchRegex()) + ->convert('branch', 'escaper.argument:escape') ->bind('searchcommits'); $route->get('{repo}/commit/{commit}', function ($repo, $commit) use ($app) { @@ -125,6 +127,7 @@ class CommitController implements ControllerProviderInterface )); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', $app['util.routing']->getCommitishPathRegex()) + ->convert('commitishPath', 'escaper.argument:escape') ->bind('blame'); return $route; diff --git a/src/GitList/Controller/MainController.php b/src/GitList/Controller/MainController.php index 3c8919c..92de6ae 100644 --- a/src/GitList/Controller/MainController.php +++ b/src/GitList/Controller/MainController.php @@ -48,6 +48,7 @@ class MainController implements ControllerProviderInterface })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('branch', $app['util.routing']->getBranchRegex()) ->value('branch', null) + ->convert('branch', 'escaper.argument:escape') ->bind('stats'); $route->get('{repo}/{branch}/rss/', function($repo, $branch) use ($app) { @@ -69,6 +70,7 @@ class MainController implements ControllerProviderInterface })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('branch', $app['util.routing']->getBranchRegex()) ->value('branch', null) + ->convert('branch', 'escaper.argument:escape') ->bind('rss'); return $route; diff --git a/src/GitList/Controller/NetworkController.php b/src/GitList/Controller/NetworkController.php index 2814e9c..1c6a920 100644 --- a/src/GitList/Controller/NetworkController.php +++ b/src/GitList/Controller/NetworkController.php @@ -55,7 +55,7 @@ class NetworkController implements ControllerProviderInterface } $nextPageUrl = null; - + if ($pager['last'] !== $pager['current']) { $nextPageUrl = $app['url_generator']->generate( 'networkData', @@ -66,10 +66,10 @@ class NetworkController implements ControllerProviderInterface ) ); } - + // when no commits are given, return an empty response - issue #369 if( count($commits) === 0 ) { - return $app->json( array( + return $app->json( array( 'repo' => $repo, 'commitishPath' => $commitishPath, 'nextPage' => null, @@ -91,6 +91,7 @@ class NetworkController implements ControllerProviderInterface )->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', $app['util.routing']->getCommitishPathRegex()) ->value('commitishPath', null) + ->convert('commitishPath', 'escaper.argument:escape') ->assert('page', '\d+') ->value('page', '0') ->bind('networkData'); @@ -119,6 +120,7 @@ class NetworkController implements ControllerProviderInterface )->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', $app['util.routing']->getCommitishPathRegex()) ->value('commitishPath', null) + ->convert('commitishPath', 'escaper.argument:escape') ->bind('network'); return $route; diff --git a/src/GitList/Controller/TreeController.php b/src/GitList/Controller/TreeController.php index 7d1369c..b3d88b4 100644 --- a/src/GitList/Controller/TreeController.php +++ b/src/GitList/Controller/TreeController.php @@ -45,6 +45,7 @@ class TreeController implements ControllerProviderInterface )); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('commitishPath', $app['util.routing']->getCommitishPathRegex()) + ->convert('commitishPath', 'escaper.argument:escape') ->bind('tree'); $route->post('{repo}/tree/{branch}/search', function (Request $request, $repo, $branch = '', $tree = '') use ($app) { @@ -69,6 +70,7 @@ class TreeController implements ControllerProviderInterface )); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('branch', $app['util.routing']->getBranchRegex()) + ->convert('branch', 'escaper.argument:escape') ->bind('search'); $route->get('{repo}/{format}ball/{branch}', function($repo, $format, $branch) use ($app) { @@ -95,6 +97,7 @@ class TreeController implements ControllerProviderInterface })->assert('format', '(zip|tar)') ->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('branch', $app['util.routing']->getBranchRegex()) + ->convert('branch', 'escaper.argument:escape') ->bind('archive'); @@ -102,6 +105,7 @@ class TreeController implements ControllerProviderInterface return $treeController($repo, $branch); })->assert('repo', $app['util.routing']->getRepositoryRegex()) ->assert('branch', $app['util.routing']->getBranchRegex()) + ->convert('branch', 'escaper.argument:escape') ->bind('branch'); $route->get('{repo}/', function($repo) use ($app, $treeController) { diff --git a/src/GitList/Escaper/ArgumentEscaper.php b/src/GitList/Escaper/ArgumentEscaper.php new file mode 100644 index 0000000..6717dd8 --- /dev/null +++ b/src/GitList/Escaper/ArgumentEscaper.php @@ -0,0 +1,15 @@ +