From 7309cefd60f4e360f1938da50d4fd32053f69969 Mon Sep 17 00:00:00 2001 From: Eldad Fux Date: Thu, 10 Sep 2020 11:33:47 +0300 Subject: [PATCH 1/3] Fixed roles param input validation --- app/controllers/api/teams.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/teams.php b/app/controllers/api/teams.php index 8981695aa3..d2e8cda0ea 100644 --- a/app/controllers/api/teams.php +++ b/app/controllers/api/teams.php @@ -18,6 +18,7 @@ use Appwrite\Database\Document; use Appwrite\Database\Validator\UID; use Appwrite\Database\Validator\Authorization; use Appwrite\Database\Exception\Duplicate; +use Appwrite\Database\Validator\Key; use Appwrite\Template\Template; $utopia->post('/v1/teams') @@ -29,7 +30,7 @@ $utopia->post('/v1/teams') ->label('sdk.method', 'create') ->label('sdk.description', '/docs/references/teams/create-team.md') ->param('name', null, function () { return new Text(100); }, 'Team name.') - ->param('roles', ['owner'], function () { return new ArrayList(new Text(128)); }, 'Array of strings. Use this param to set the roles in the team for the user who created it. The default role is **owner**. A role can be any string. Learn more about [roles and permissions](/docs/permissions).', true) + ->param('roles', ['owner'], function () { return new ArrayList(new Key()); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](/docs/permissions). Max length for each role is 32 chars.') ->action( function ($name, $roles) use ($response, $projectDB, $user, $mode) { Authorization::disable(); @@ -216,7 +217,7 @@ $utopia->post('/v1/teams/:teamId/memberships') ->param('teamId', '', function () { return new UID(); }, 'Team unique ID.') ->param('email', '', function () { return new Email(); }, 'New team member email.') ->param('name', '', function () { return new Text(100); }, 'New team member name.', true) - ->param('roles', [], function () { return new ArrayList(new Text(128)); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](/docs/permissions).') + ->param('roles', [], function () { return new ArrayList(new Key()); }, 'Array of strings. Use this param to set the user roles in the team. A role can be any string. Learn more about [roles and permissions](/docs/permissions). Max length for each role is 32 chars.') ->param('url', '', function () use ($clients) { return new Host($clients); }, 'URL to redirect the user back to your app from the invitation email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.') // TODO add our own built-in confirm page ->action( function ($teamId, $email, $name, $roles, $url) use ($response, $mail, $project, $user, $audit, $projectDB, &$mode) { From 42341fddbb4d3f83a5f82ec59ed4db6f9f6e100c Mon Sep 17 00:00:00 2001 From: "Eldad A. Fux" Date: Thu, 10 Sep 2020 13:07:28 +0300 Subject: [PATCH 2/3] Create codeql-analysis.yml --- .github/workflows/codeql-analysis.yml | 62 +++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 .github/workflows/codeql-analysis.yml diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 0000000000..a0b044ecf2 --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,62 @@ +name: "CodeQL" + +on: + push: + branches: [master] + pull_request: + # The branches below must be a subset of the branches above + branches: [master] + schedule: + - cron: '0 16 * * 0' + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + # Override automatic language detection by changing the below list + # Supported options are ['csharp', 'cpp', 'go', 'java', 'javascript', 'python'] + language: ['javascript'] + # Learn more... + # https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#overriding-automatic-language-detection + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + # We must fetch at least the immediate parents so that if this is + # a pull request then we can checkout the head. + fetch-depth: 2 + + # If this run was triggered by a pull request event, then checkout + # the head of the pull request instead of the merge commit. + - run: git checkout HEAD^2 + if: ${{ github.event_name == 'pull_request' }} + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: ${{ matrix.language }} + + # Autobuild attempts to build any compiled languages (C/C++, C#, or Java). + # If this step fails, then you should remove it and run the build manually (see below) + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + # ℹī¸ Command-line programs to run using the OS shell. + # 📚 https://git.io/JvXDl + + # ✏ī¸ If the Autobuild fails above, remove it and uncomment the following three lines + # and modify them (or add more) to build your code if your project + # uses a compiled language + + #- run: | + # make bootstrap + # make release + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 From 76dfa7531825c30b9bb53126bbc1c1554a3fa0b9 Mon Sep 17 00:00:00 2001 From: Eldad Fux Date: Fri, 11 Sep 2020 09:05:21 +0300 Subject: [PATCH 3/3] Cleaned up unused vars --- app/controllers/api/account.php | 10 ++++------ app/controllers/api/database.php | 20 ++++++++------------ app/controllers/api/storage.php | 12 +++++++----- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 6f63670331..8618bb1379 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -41,12 +41,11 @@ App::post('/v1/account') ->param('email', '', function () { return new Email(); }, 'User email.') ->param('password', '', function () { return new Password(); }, 'User password. Must be between 6 to 32 chars.') ->param('name', '', function () { return new Text(128); }, 'User name. Max length: 128 chars.', true) - ->action(function ($email, $password, $name, $request, $response, $project, $projectDB, $webhooks, $audits) { + ->action(function ($email, $password, $name, $request, $response, $project, $projectDB, $audits) { /** @var Utopia\Swoole\Request $request */ /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Document $project */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ if ('console' === $project->getId()) { @@ -119,7 +118,7 @@ App::post('/v1/account') $response->setStatusCode(Response::STATUS_CODE_CREATED); $response->dynamic($user, Response::MODEL_USER); - }, ['request', 'response', 'project', 'projectDB', 'webhooks', 'audits']); + }, ['request', 'response', 'project', 'projectDB', 'audits']); App::post('/v1/account/sessions') ->desc('Create Account Session') @@ -134,13 +133,12 @@ App::post('/v1/account/sessions') ->label('abuse-key', 'url:{url},email:{param-email}') ->param('email', '', function () { return new Email(); }, 'User email.') ->param('password', '', function () { return new Password(); }, 'User password. Must be between 6 to 32 chars.') - ->action(function ($email, $password, $request, $response, $projectDB, $locale, $geodb, $webhooks, $audits) { + ->action(function ($email, $password, $request, $response, $projectDB, $locale, $geodb, $audits) { /** @var Utopia\Swoole\Request $request */ /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Database $projectDB */ /** @var Utopia\Locale\Locale $locale */ /** @var GeoIp2\Database\Reader $geodb */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ $protocol = $request->getProtocol(); @@ -255,7 +253,7 @@ App::post('/v1/account/sessions') ; $response->dynamic($session, Response::MODEL_SESSION); - }, ['request', 'response', 'projectDB', 'locale', 'geodb', 'webhooks', 'audits']); + }, ['request', 'response', 'projectDB', 'locale', 'geodb', 'audits']); App::get('/v1/account/sessions/oauth2/:provider') ->desc('Create Account Session with OAuth2') diff --git a/app/controllers/api/database.php b/app/controllers/api/database.php index 45385034ab..0300fcfc47 100644 --- a/app/controllers/api/database.php +++ b/app/controllers/api/database.php @@ -34,10 +34,9 @@ App::post('/v1/database/collections') ->param('read', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with read permissions. By default no user is granted with any read permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->param('write', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with write permissions. By default no user is granted with any write permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->param('rules', [], function ($projectDB) { return new ArrayList(new Collection($projectDB, [Database::SYSTEM_COLLECTION_RULES], ['$collection' => Database::SYSTEM_COLLECTION_RULES, '$permissions' => ['read' => [], 'write' => []]])); }, 'Array of [rule objects](/docs/rules). Each rule define a collection field name, data type and validation.', false, ['projectDB']) - ->action(function ($name, $read, $write, $rules, $response, $projectDB, $webhooks, $audits) { + ->action(function ($name, $read, $write, $rules, $response, $projectDB, $audits) { /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ $parsedRules = []; @@ -85,7 +84,7 @@ App::post('/v1/database/collections') $response->setStatusCode(Response::STATUS_CODE_CREATED); $response->dynamic($data, Response::MODEL_COLLECTION); - }, ['response', 'projectDB', 'webhooks', 'audits']); + }, ['response', 'projectDB', 'audits']); App::get('/v1/database/collections') ->desc('List Collections') @@ -157,10 +156,9 @@ App::put('/v1/database/collections/:collectionId') ->param('read', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with read permissions. By default no user is granted with any read permissions. [learn more about permissions(/docs/permissions) and get a full list of available permissions.') ->param('write', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with write permissions. By default no user is granted with any write permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->param('rules', [], function ($projectDB) { return new ArrayList(new Collection($projectDB, [Database::SYSTEM_COLLECTION_RULES], ['$collection' => Database::SYSTEM_COLLECTION_RULES, '$permissions' => ['read' => [], 'write' => []]])); }, 'Array of [rule objects](/docs/rules). Each rule define a collection field name, data type and validation.', true, ['projectDB']) - ->action(function ($collectionId, $name, $read, $write, $rules, $response, $projectDB, $webhooks, $audits) { + ->action(function ($collectionId, $name, $read, $write, $rules, $response, $projectDB, $audits) { /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ $collection = $projectDB->getDocument($collectionId, false); @@ -211,7 +209,7 @@ App::put('/v1/database/collections/:collectionId') ; $response->dynamic($collection, Response::MODEL_COLLECTION); - }, ['response', 'projectDB', 'webhooks', 'audits']); + }, ['response', 'projectDB', 'audits']); App::delete('/v1/database/collections/:collectionId') ->desc('Delete Collection') @@ -268,10 +266,9 @@ App::post('/v1/database/collections/:collectionId/documents') ->param('parentDocument', '', function () { return new UID(); }, 'Parent document unique ID. Use when you want your new document to be a child of a parent document.', true) ->param('parentProperty', '', function () { return new Key(); }, 'Parent document property name. Use when you want your new document to be a child of a parent document.', true) ->param('parentPropertyType', Document::SET_TYPE_ASSIGN, function () { return new WhiteList([Document::SET_TYPE_ASSIGN, Document::SET_TYPE_APPEND, Document::SET_TYPE_PREPEND], true); }, 'Parent document property connection type. You can set this value to **assign**, **append** or **prepend**, default value is assign. Use when you want your new document to be a child of a parent document.', true) - ->action(function ($collectionId, $data, $read, $write, $parentDocument, $parentProperty, $parentPropertyType, $response, $projectDB, $webhooks, $audits) { + ->action(function ($collectionId, $data, $read, $write, $parentDocument, $parentProperty, $parentPropertyType, $response, $projectDB, $audits) { /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ $data = (\is_string($data)) ? \json_decode($data, true) : $data; // Cast to JSON array @@ -366,7 +363,7 @@ App::post('/v1/database/collections/:collectionId/documents') ; $response->dynamic($data, Response::MODEL_ANY); - }, ['response', 'projectDB', 'webhooks', 'audits']); + }, ['response', 'projectDB', 'audits']); App::get('/v1/database/collections/:collectionId/documents') ->desc('List Documents') @@ -465,10 +462,9 @@ App::patch('/v1/database/collections/:collectionId/documents/:documentId') ->param('data', [], function () { return new JSON(); }, 'Document data as JSON object.') ->param('read', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with read permissions. By default no user is granted with any read permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->param('write', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with write permissions. By default no user is granted with any write permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') - ->action(function ($collectionId, $documentId, $data, $read, $write, $response, $projectDB, $webhooks, $audits) { + ->action(function ($collectionId, $documentId, $data, $read, $write, $response, $projectDB, $audits) { /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ $collection = $projectDB->getDocument($collectionId, false); @@ -524,7 +520,7 @@ App::patch('/v1/database/collections/:collectionId/documents/:documentId') ; $response->dynamic($data, Response::MODEL_ANY); - }, ['response', 'projectDB', 'webhooks', 'audits']); + }, ['response', 'projectDB', 'audits']); App::delete('/v1/database/collections/:collectionId/documents/:documentId') ->desc('Delete Document') diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index f519a74342..b39d109dd3 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -37,12 +37,11 @@ App::post('/v1/storage/files') ->param('file', [], function () { return new File(); }, 'Binary file.', false) ->param('read', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with read permissions. By default no user is granted with any read permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->param('write', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with write permissions. By default no user is granted with any write permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') - ->action(function ($file, $read, $write, $request, $response, $user, $projectDB, $webhooks, $audits, $usage) { + ->action(function ($file, $read, $write, $request, $response, $user, $projectDB, $audits, $usage) { /** @var Utopia\Swoole\Request $request */ /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Document $user */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ /** @var Appwrite\Event\Event $usage */ @@ -152,7 +151,7 @@ App::post('/v1/storage/files') $response->setStatusCode(Response::STATUS_CODE_CREATED); $response->dynamic($file, Response::MODEL_FILE); - }, ['request', 'response', 'user', 'projectDB', 'webhooks', 'audits', 'usage']); + }, ['request', 'response', 'user', 'projectDB', 'audits', 'usage']); App::get('/v1/storage/files') ->desc('List Files') @@ -476,10 +475,9 @@ App::put('/v1/storage/files/:fileId') ->param('fileId', '', function () { return new UID(); }, 'File unique ID.') ->param('read', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with read permissions. By default no user is granted with any read permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') ->param('write', [], function () { return new ArrayList(new Text(64)); }, 'An array of strings with write permissions. By default no user is granted with any write permissions. [learn more about permissions](/docs/permissions) and get a full list of available permissions.') - ->action(function ($fileId, $read, $write, $response, $projectDB, $webhooks, $audits) { + ->action(function ($fileId, $read, $write, $response, $projectDB, $audits) { /** @var Appwrite\Utopia\Response $response */ /** @var Appwrite\Database\Database $projectDB */ - /** @var Appwrite\Event\Event $webhooks */ /** @var Appwrite\Event\Event $audits */ $file = $projectDB->getDocument($fileId); @@ -548,6 +546,10 @@ App::delete('/v1/storage/files/:fileId') ->setParam('storage', $file->getAttribute('size', 0) * -1) ; + $webhooks + ->setParam('payload', $response->output($file, Response::MODEL_FILE)) + ; + $response->noContent(); }, ['response', 'projectDB', 'webhooks', 'audits', 'usage']);