diff --git a/extension/chrome/dev/ci_unit_test.ts b/extension/chrome/dev/ci_unit_test.ts index 190b69ebc80..fb09eb6f9cc 100644 --- a/extension/chrome/dev/ci_unit_test.ts +++ b/extension/chrome/dev/ci_unit_test.ts @@ -8,6 +8,7 @@ import { AttachmentUI } from '../../js/common/ui/attachment-ui.js'; import { ApiErr } from '../../js/common/api/shared/api-error.js'; import { Mime } from '../../js/common/core/mime.js'; import { Attachment } from '../../js/common/core/attachment.js'; +import { Attester } from '../../js/common/api/key-server/attester.js'; import { Wkd } from '../../js/common/api/key-server/wkd.js'; import { MsgUtil } from '../../js/common/core/crypto/pgp/msg-util.js'; import { Sks } from '../../js/common/api/key-server/sks.js'; @@ -32,6 +33,7 @@ const libs: unknown[] = [ ApiErr, Attachment, AttachmentUI, + Attester, Buf, ExpirationCache, KeyUtil, diff --git a/extension/chrome/settings/index.ts b/extension/chrome/settings/index.ts index 81d18a933a8..eb1338a0c6b 100644 --- a/extension/chrome/settings/index.ts +++ b/extension/chrome/settings/index.ts @@ -163,8 +163,7 @@ View.run( this.setHandler(async () => { const prvs = await KeyStoreUtil.parse(await KeyStore.getRequired(this.acctEmail!)); const mostUsefulPrv = KeyStoreUtil.chooseMostUseful(prvs, 'EVEN-IF-UNUSABLE'); - const escapedFp = Xss.escape(mostUsefulPrv!.key.id); - await Settings.renderSubPage(this.acctEmail, this.tabId, 'modules/my_key.htm', `&fingerprint=${escapedFp}`); + await Settings.renderSubPage(this.acctEmail, this.tabId, 'modules/my_key.htm', { fingerprint: mostUsefulPrv!.key.id }); }) ); $('.action_show_encrypted_inbox').on( @@ -538,7 +537,7 @@ View.run( } const escapedEmail = Xss.escape(KeyUtil.getPrimaryEmail(prv) || ''); - const escapedLink = `${escapedEmail}`; + const escapedLink = `${escapedEmail}`; const fpHtml = `${Str.spaced(Xss.escape(ki.fingerprints[0]))}`; const rowClass = isExpired ? 'key-content-row expired' : 'key-content-row'; diff --git a/extension/js/common/api/account-servers/external-service.ts b/extension/js/common/api/account-servers/external-service.ts index 619ed6096e2..e3c65ef184d 100644 --- a/extension/js/common/api/account-servers/external-service.ts +++ b/extension/js/common/api/account-servers/external-service.ts @@ -5,7 +5,7 @@ import { AuthenticationConfiguration } from '../../authentication-configuration. import { ClientConfigurationError, ClientConfigurationJson } from '../../client-configuration.js'; import { Attachment } from '../../core/attachment.js'; import { Buf } from '../../core/buf.js'; -import { Dict, Str } from '../../core/common.js'; +import { Dict, Str, Url } from '../../core/common.js'; import { FLAVOR, SHARED_TENANT_API_HOST } from '../../core/const.js'; import { ErrorReport } from '../../platform/error-report.js'; import { Serializable } from '../../platform/store/abstract-store.js'; @@ -95,13 +95,18 @@ export class ExternalService extends Api { public fetchAndSaveClientConfiguration = async (): Promise => { const auth = await this.request( - `/api/${this.apiVersion}/client-configuration/authentication?domain=${this.domain}`, + Url.create(`/api/${this.apiVersion}/client-configuration/authentication`, { domain: this.domain }), undefined, undefined, false ); await AcctStore.set(this.acctEmail, { authentication: auth }); - const r = await this.request(`/api/${this.apiVersion}/client-configuration?domain=${this.domain}`, undefined, undefined, false); + const r = await this.request( + Url.create(`/api/${this.apiVersion}/client-configuration`, { domain: this.domain }), + undefined, + undefined, + false + ); if (r.clientConfiguration && !r.clientConfiguration.flags) { throw new ClientConfigurationError('missing_flags'); } @@ -173,7 +178,7 @@ export class ExternalService extends Api { }; public messageGatewayUpdate = async (externalId: string, emailGatewayMessageId: string) => { - await this.request(`/api/${this.apiVersion}/message/${externalId}/gateway`, { + await this.request(`/api/${this.apiVersion}/message/${encodeURIComponent(externalId)}/gateway`, { fmt: 'JSON', data: { emailGatewayMessageId, @@ -278,5 +283,5 @@ export class ExternalService extends Api { cc: process(recipients.cc), bcc: process(recipients.bcc), }; - } + }; } diff --git a/extension/js/common/api/authentication/google/google-oauth.ts b/extension/js/common/api/authentication/google/google-oauth.ts index 784786bdf5c..19d062d8d77 100644 --- a/extension/js/common/api/authentication/google/google-oauth.ts +++ b/extension/js/common/api/authentication/google/google-oauth.ts @@ -41,7 +41,7 @@ export class GoogleOAuth extends OAuth { public static async getTokenInfo(accessToken: string): Promise { return await Api.ajax( { - url: `${OAUTH_GOOGLE_API_HOST}/tokeninfo?access_token=${accessToken}`, + url: `${OAUTH_GOOGLE_API_HOST}/tokeninfo?${new URLSearchParams([['access_token', accessToken]]).toString()}`, method: 'GET', timeout: 10000, stack: CatchHelper.stackTrace(), diff --git a/extension/js/common/api/email-provider/gmail/gmail.ts b/extension/js/common/api/email-provider/gmail/gmail.ts index b62c2453a06..195305ed26e 100644 --- a/extension/js/common/api/email-provider/gmail/gmail.ts +++ b/extension/js/common/api/email-provider/gmail/gmail.ts @@ -36,7 +36,12 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { public threadGet = async (threadId: string, format?: GmailResponseFormat, progressCb?: ProgressCb, retryCount = 0): Promise => { try { - return await Google.gmailCall(this.acctEmail, `threads/${threadId}`, { method: 'GET', data: { format } }, { download: progressCb }); + return await Google.gmailCall( + this.acctEmail, + `threads/${encodeURIComponent(threadId)}`, + { method: 'GET', data: { format } }, + { download: progressCb } + ); } catch (e) { if (ApiErr.isRateLimit(e) && retryCount < MAX_RATE_LIMIT_ERROR_RETRY_COUNT) { await Time.sleep(1000); @@ -60,7 +65,7 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { }; public threadModify = async (id: string, rmLabels: string[], addLabels: string[]): Promise => { - return await Google.gmailCall(this.acctEmail, `threads/${id}/modify`, { + return await Google.gmailCall(this.acctEmail, `threads/${encodeURIComponent(id)}/modify`, { method: 'POST', data: { removeLabelIds: rmLabels || [], // todo - insufficient permission - need https://github.com/FlowCrypt/flowcrypt-browser/issues/1304 @@ -79,11 +84,11 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { }; public draftDelete = async (id: string): Promise => { - return await Google.gmailCall(this.acctEmail, 'drafts/' + id, { method: 'DELETE' }); + return await Google.gmailCall(this.acctEmail, `drafts/${encodeURIComponent(id)}`, { method: 'DELETE' }); }; public draftUpdate = async (id: string, mimeMsg: string, threadId: string): Promise => { - return await Google.gmailCall(this.acctEmail, `drafts/${id}`, { + return await Google.gmailCall(this.acctEmail, `drafts/${encodeURIComponent(id)}`, { method: 'PUT', data: { message: { raw: Buf.fromUtfStr(mimeMsg).toBase64UrlStr(), threadId }, @@ -92,7 +97,7 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { }; public draftGet = async (id: string, format: GmailResponseFormat = 'full'): Promise => { - return await Google.gmailCall(this.acctEmail, `drafts/${id}`, { method: 'GET', data: { format } }); + return await Google.gmailCall(this.acctEmail, `drafts/${encodeURIComponent(id)}`, { method: 'GET', data: { format } }); }; public draftList = async (): Promise => { @@ -139,7 +144,7 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { try { return await Google.gmailCall( this.acctEmail, - `messages/${msgId}`, + `messages/${encodeURIComponent(msgId)}`, { method: 'GET', data: { format: format || 'full' } }, progressCb ? { download: progressCb } : undefined ); @@ -164,10 +169,13 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { }; public attachmentGet = async (attachment: Attachment, progress?: { download: ProgressCb }): Promise => { + if (!attachment.msgId || !attachment.id) { + throw new Error('Attachment is missing Gmail message id or attachment id'); + } type RawGmailAttRes = { attachmentId: string; size: number; data: string }; const { attachmentId, size, data } = await Google.gmailCall( this.acctEmail, - `messages/${attachment.msgId}/attachments/${attachment.id}`, + `messages/${encodeURIComponent(attachment.msgId)}/attachments/${encodeURIComponent(attachment.id)}`, { method: 'GET' }, progress ); @@ -227,7 +235,7 @@ export class Gmail extends EmailProviderApi implements EmailProviderInterface { }; GoogleOAuth.googleApiAuthHeader(this.acctEmail) .then(async authHeader => { - const url = `${GMAIL_GOOGLE_API_HOST}/gmail/v1/users/me/messages/${msgId}/attachments/${attachmentId}`; + const url = `${GMAIL_GOOGLE_API_HOST}/gmail/v1/users/me/messages/${encodeURIComponent(msgId)}/attachments/${encodeURIComponent(attachmentId)}`; const response: Response = await fetch(url, { method: 'GET', headers: authHeader, diff --git a/extension/js/common/api/email-provider/gmail/google.ts b/extension/js/common/api/email-provider/gmail/google.ts index 819f33a918e..13a9391056e 100644 --- a/extension/js/common/api/email-provider/gmail/google.ts +++ b/extension/js/common/api/email-provider/gmail/google.ts @@ -11,7 +11,7 @@ import { GoogleOAuth } from '../../authentication/google/google-oauth.js'; import { CatchHelper } from '../../../platform/catch-helper.js'; export class Google { public static webmailUrl = (acctEmail: string) => { - return `https://mail.google.com/mail/u/${acctEmail}`; + return `https://mail.google.com/mail/u/${encodeURIComponent(acctEmail)}`; }; public static gmailCall = async (acctEmail: string, path: string, params?: AjaxParams, progress?: ProgressCbs): Promise => { diff --git a/extension/js/common/api/flowcrypt-website.ts b/extension/js/common/api/flowcrypt-website.ts index 1291c958017..5ddc5fd6357 100644 --- a/extension/js/common/api/flowcrypt-website.ts +++ b/extension/js/common/api/flowcrypt-website.ts @@ -11,7 +11,7 @@ namespace FlowCryptWebsiteRes { export class FlowCryptWebsite extends Api { public static pubKeyUrl = (resource: string) => { - return `https://flowcrypt.com/pub/${resource}`; + return `https://flowcrypt.com/pub/${encodeURIComponent(resource)}`; }; public static retrieveBlogPosts = async (): Promise => { diff --git a/extension/js/common/api/key-server/attester.ts b/extension/js/common/api/key-server/attester.ts index cce4cdb1c7e..d1d185679e4 100644 --- a/extension/js/common/api/key-server/attester.ts +++ b/extension/js/common/api/key-server/attester.ts @@ -42,7 +42,8 @@ export class Attester extends Api { public doLookupLdap = async (email: string, server?: string): Promise => { const ldapServer = server ?? `keys.${Str.getDomainFromEmailAddress(email)}`; try { - const r = await this.pubCall(`ldap-relay?server=${ldapServer}&search=${email}`); + const query = new URLSearchParams({ server: ldapServer, search: email }); + const r = await this.pubCall(`ldap-relay?${query.toString()}`); return await this.getPubKeysSearchResult(r); } catch (e) { // treat error 500 as error 404 on this particular endpoint @@ -73,7 +74,7 @@ export class Attester extends Api { if (!this.clientConfiguration.canSubmitPubToAttester()) { throw new Error('Cannot replace pubkey at attester because your organisation rules forbid it'); } - await this.pubCall(`pub/${email}`, pubkey, { authorization: `Bearer ${idToken}` }); + await this.pubCall(`pub/${encodeURIComponent(email)}`, pubkey, { authorization: `Bearer ${idToken}` }); }; /** @@ -85,7 +86,7 @@ export class Attester extends Api { if (!this.clientConfiguration.canSubmitPubToAttester()) { throw new Error('Cannot replace pubkey at attester because your organisation rules forbid it'); } - return await this.pubCall(`pub/${email}`, pubkey); + return await this.pubCall(`pub/${encodeURIComponent(email)}`, pubkey); }; public welcomeMessage = async (email: string, pubkey: string, idToken: string | undefined): Promise<{ sent: boolean }> => { @@ -109,7 +110,7 @@ export class Attester extends Api { private doLookup = async (email: string): Promise => { try { - const r = await this.pubCall(`pub/${email}`); + const r = await this.pubCall(`pub/${encodeURIComponent(email)}`); return await this.getPubKeysSearchResult(r); } catch (e) { if (ApiErr.isNotFound(e)) { diff --git a/extension/js/common/browser/ui.ts b/extension/js/common/browser/ui.ts index 26df8fe7ace..f6ed7694ba4 100644 --- a/extension/js/common/browser/ui.ts +++ b/extension/js/common/browser/ui.ts @@ -367,7 +367,7 @@ export class Ui { }; public static getTestCompatibilityLink = (acctEmail: string): string => { - return `Test your OpenPGP key compatibility`; + return `Test your OpenPGP key compatibility`; }; public static getScreenDimensions = (): ScreenDimensions => { diff --git a/extension/js/service_worker/bg-handlers.ts b/extension/js/service_worker/bg-handlers.ts index fc5657ec7f8..7a68854dd43 100644 --- a/extension/js/service_worker/bg-handlers.ts +++ b/extension/js/service_worker/bg-handlers.ts @@ -182,6 +182,9 @@ export class BgHandlers { }; public static thunderbirdOpenPassphraseDialog = async (r: Bm.ThunderbirdOpenPassphraseDialog): Promise => { - await BgUtils.openExtensionTab(`chrome/elements/passphrase.htm?type=message&parentTabId=0&acctEmail=${r.acctEmail}&longids=${r.longids}`, true); + await BgUtils.openExtensionTab( + Url.create('chrome/elements/passphrase.htm', { type: 'message', parentTabId: '0', acctEmail: r.acctEmail, longids: r.longids }), + true + ); }; } diff --git a/test/source/mock/attester/attester-endpoints.ts b/test/source/mock/attester/attester-endpoints.ts index 16326420bcf..11391baa264 100644 --- a/test/source/mock/attester/attester-endpoints.ts +++ b/test/source/mock/attester/attester-endpoints.ts @@ -24,7 +24,7 @@ export interface AttesterConfig { export const getMockAttesterEndpoints = (oauth: OauthMock, attesterConfig: AttesterConfig): HandlersDefinition => { return { '/attester/pub/?': async ({ body }, req) => { - const emailOrLongid = req.url.split('/').pop()!.toLowerCase().trim(); + const emailOrLongid = decodeURIComponent(req.url.split('/').pop()!).toLowerCase().trim(); if (isGet(req)) { if (!attesterConfig?.pubkeyLookup) { throw new HttpClientErr('Method not allowed', 405); diff --git a/test/source/mock/fes/customer-url-fes-endpoints.ts b/test/source/mock/fes/customer-url-fes-endpoints.ts index 4766ec12c08..b02a6303078 100644 --- a/test/source/mock/fes/customer-url-fes-endpoints.ts +++ b/test/source/mock/fes/customer-url-fes-endpoints.ts @@ -106,7 +106,7 @@ export const getMockCustomerUrlFesEndpoints = (config: FesConfig | undefined): H const port = parsePort(req); const gatewayMatch = /\/api\/v1\/messages\/([^/]+)\/gateway/.exec(req.url); if (gatewayMatch && parseAuthority(req) === standardFesUrl(port) && req.method === 'POST') { - const externalId = gatewayMatch[1]; + const externalId = decodeURIComponent(gatewayMatch[1]); if (externalId === 'FES-MOCK-EXTERNAL-FOR-GATEWAYFAILURE@EXAMPLE.COM-ID') { throw new HttpClientErr(`Test error`, Status.BAD_REQUEST); } @@ -141,7 +141,7 @@ export const getMockCustomerUrlFesEndpoints = (config: FesConfig | undefined): H const port = parsePort(req); const gatewayMatch = /\/api\/v1\/message\/([^/]+)\/gateway/.exec(req.url); if (gatewayMatch && parseAuthority(req) === standardFesUrl(port) && req.method === 'POST') { - const externalId = gatewayMatch[1]; + const externalId = decodeURIComponent(gatewayMatch[1]); if (externalId === 'FES-MOCK-EXTERNAL-FOR-GATEWAYFAILURE@EXAMPLE.COM-ID') { throw new HttpClientErr(`Test error`, Status.BAD_REQUEST); } diff --git a/test/source/mock/fes/shared-tenant-fes-endpoints.ts b/test/source/mock/fes/shared-tenant-fes-endpoints.ts index 896cdf580d2..0d4eac64073 100644 --- a/test/source/mock/fes/shared-tenant-fes-endpoints.ts +++ b/test/source/mock/fes/shared-tenant-fes-endpoints.ts @@ -214,7 +214,7 @@ export const getMockSharedTenantFesEndpoints = (config: FesConfig | undefined): '/shared-tenant-fes/api/v1/messages/?': async ({ body }, req) => { const gatewayMatch = /\/shared-tenant-fes\/api\/v1\/messages\/([^/]+)\/gateway/.exec(req.url); if (gatewayMatch && req.method === 'POST') { - const externalId = gatewayMatch[1]; + const externalId = decodeURIComponent(gatewayMatch[1]); if (externalId === 'FES-MOCK-EXTERNAL-FOR-GATEWAYFAILURE@EXAMPLE.COM-ID') { throw new HttpClientErr(`Test error`, Status.BAD_REQUEST); } @@ -253,7 +253,7 @@ export const getMockSharedTenantFesEndpoints = (config: FesConfig | undefined): '/shared-tenant-fes/api/v1/message/?': async ({ body }, req) => { const gatewayMatch = /\/shared-tenant-fes\/api\/v1\/message\/([^/]+)\/gateway/.exec(req.url); if (gatewayMatch && req.method === 'POST') { - const externalId = gatewayMatch[1]; + const externalId = decodeURIComponent(gatewayMatch[1]); if (externalId === 'FES-MOCK-EXTERNAL-FOR-GATEWAYFAILURE@EXAMPLE.COM-ID') { throw new HttpClientErr(`Test error`, Status.BAD_REQUEST); } diff --git a/test/source/tests/browser-unit-tests/unit-Attester.js b/test/source/tests/browser-unit-tests/unit-Attester.js new file mode 100644 index 00000000000..3c552e3b617 --- /dev/null +++ b/test/source/tests/browser-unit-tests/unit-Attester.js @@ -0,0 +1,35 @@ +/* ©️ 2016 - present FlowCrypt a.s. Limitations apply. Contact human@flowcrypt.com */ + +/** + * These tests use JavaScript instead of TypeScript to avoid dealing with types in cross-environment setup. + * (tests are injected from NodeJS through puppeteer into a browser environment) + * While this makes them less convenient to write, the result is more flexible. + * + * Import your lib to `ci_unit_test.ts` to resolve `ReferenceError: SomeClass is not defined` + * + * Each test must return "pass" to pass. To reject, throw an Error. + * + * Each test must start with one of (depending on which flavors you want it to run): + * - BROWSER_UNIT_TEST_NAME(`some test name`); + * - BROWSER_UNIT_TEST_NAME(`some test name`).enterprise; + * - BROWSER_UNIT_TEST_NAME(`some test name`).consumer; + * + * This is not a JavaScript file. It's a text file that gets parsed, split into chunks, and + * parts of it executed as javascript. The structure is very rigid. The only flexible place is inside + * the async functions. For the rest, do not change the structure or our parser will get confused. + * Do not put any code whatsoever outside of the async functions. + */ + +BROWSER_UNIT_TEST_NAME(`Attester lookup encodes email path segment`); +(async () => { + const maliciousEmail = '%2e%2e/ldap-relay?server=attacker.example&search=x@example.com@flowcrypt.test'; + const attester = new Attester({ + canLookupThisRecipientOnAttester: () => true, + canSubmitPubToAttester: () => true, + }); + const { pubkeys } = await attester.lookupEmail(maliciousEmail); + if (pubkeys.length) { + throw Error(`Expected no pubkey for encoded path traversal email, got ${pubkeys.length}`); + } + return 'pass'; +})(); diff --git a/test/source/tests/unit-browser.ts b/test/source/tests/unit-browser.ts index ffd9980fdb6..73dfcfbd48f 100644 --- a/test/source/tests/unit-browser.ts +++ b/test/source/tests/unit-browser.ts @@ -59,6 +59,15 @@ export const defineUnitBrowserTests = (testVariant: TestVariant, testWithBrowser returnError: new HttpClientErr('Pubkey not found', Status.NOT_FOUND), }, }, + attester: { + ldapRelay: { + 'x@example.com@flowcrypt.test': { + pubkey: somePubkey, + domainToCheck: 'attacker.example', + }, + }, + pubkeyLookup: {}, + }, }); } /* The "test" parameter is utilized by the unit test named "Catcher does not include query string on report." */ diff --git a/tsconfig.json b/tsconfig.json index 62dc9d7c0cb..0aded9eb57b 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,7 @@ "strict": true, "useUnknownInCatchVariables": false, "outDir": "build/generic-extension-wip", + "rootDir": "./extension", "moduleResolution": "bundler", "skipLibCheck": true, "paths": {