From 7463c98e115b11ad6bd22977f2e2f683f509e348 Mon Sep 17 00:00:00 2001 From: Pavel White Date: Mon, 25 Nov 2024 13:19:25 +0300 Subject: [PATCH] hotfix(condo): new semgrep rules (#5532) * hotfix(condo): extract share data utils * test(condo): old links should work * test(condo): add backward compatibility * fix(callsenter): fix semgrep new rules * fix(condo): use old encryption because it's better to be backward compatible * refactor(condorb): fix semgrep rules * refactor(telegram-employee-bot): fix semgrep * test(condorb): fix encryption in test --------- Co-authored-by: YEgorLu --- apps/callcenter | 2 +- .../ticket/components/ShareTicketModal.tsx | 9 +-- apps/condo/domains/ticket/constants/crypto.js | 3 + .../ticket/schema/ShareTicketService.js | 1 + .../ticket/utils/shareDataPacker.spec.js | 52 +++++++++++++++ .../domains/ticket/utils/shareDataPacker.ts | 66 +++++++++++++++++++ apps/condo/pages/share.tsx | 15 ++--- apps/condorb | 2 +- apps/telegram-employee-bot | 2 +- bin/run-semgrep.sh | 4 +- 10 files changed, 134 insertions(+), 22 deletions(-) create mode 100644 apps/condo/domains/ticket/utils/shareDataPacker.spec.js create mode 100644 apps/condo/domains/ticket/utils/shareDataPacker.ts diff --git a/apps/callcenter b/apps/callcenter index 8d7b88fbcb7..a0f921f7e71 160000 --- a/apps/callcenter +++ b/apps/callcenter @@ -1 +1 @@ -Subproject commit 8d7b88fbcb7d2bf96f38ebb5eb8321927a049fc1 +Subproject commit a0f921f7e713dfa7a17999783fff4be4bd67caf7 diff --git a/apps/condo/domains/ticket/components/ShareTicketModal.tsx b/apps/condo/domains/ticket/components/ShareTicketModal.tsx index 7ea1a46d334..b85e4da5210 100644 --- a/apps/condo/domains/ticket/components/ShareTicketModal.tsx +++ b/apps/condo/domains/ticket/components/ShareTicketModal.tsx @@ -1,6 +1,4 @@ /** @jsx jsx */ -import crypto from 'crypto' - import { green } from '@ant-design/colors' import { CloseCircleFilled, RightOutlined } from '@ant-design/icons' import { Organization as IOrganization } from '@app/condo/schema' @@ -22,9 +20,9 @@ import { useTracking, TrackingEventType } from '@condo/domains/common/components import { EN_LOCALE } from '@condo/domains/common/constants/locale' import { colors } from '@condo/domains/common/constants/style' import { getClientSideSenderInfo } from '@condo/domains/common/utils/userid.utils' -import { ALGORITHM, CRYPTOENCODING, SALT } from '@condo/domains/ticket/constants/crypto' import { SHARE_TICKET_MUTATION } from '@condo/domains/ticket/gql' import { getEmployeeWithEmail } from '@condo/domains/ticket/utils/clientSchema/search' +import { packShareData } from '@condo/domains/ticket/utils/shareDataPacker' const collapse = css` @@ -176,14 +174,13 @@ export const ShareTicketModal: React.FC = (props) => { const { logEvent, getEventName } = useTracking() const { date, number, details, id, locale, organization } = props - const cipher = crypto.createCipher(ALGORITHM, SALT) let cutDetails = details || '' if (cutDetails.length >= 110) { cutDetails = `${cutDetails.substr(0, 100)}…` } - const stringifiedParams = JSON.stringify({ date, number, details: cutDetails, id }) - const encryptedText = cipher.update(stringifiedParams, 'utf8', CRYPTOENCODING) + cipher.final(CRYPTOENCODING) + const shareParams = JSON.stringify({ date, number, details: cutDetails, id }) + const encryptedText = packShareData(shareParams) const { query } = useRouter() const [shareTicket] = useMutation(SHARE_TICKET_MUTATION) diff --git a/apps/condo/domains/ticket/constants/crypto.js b/apps/condo/domains/ticket/constants/crypto.js index d491130d54d..108e4a7cc84 100644 --- a/apps/condo/domains/ticket/constants/crypto.js +++ b/apps/condo/domains/ticket/constants/crypto.js @@ -1,5 +1,8 @@ +/** @deprecated we will drop this const soon! */ const ALGORITHM = 'aes256' +/** @deprecated we will drop this const soon! */ const SALT = '900150983cd24fb0d6963f7d28e17f72' +/** @deprecated we will drop this const soon! */ const CRYPTOENCODING = 'base64' module.exports = { diff --git a/apps/condo/domains/ticket/schema/ShareTicketService.js b/apps/condo/domains/ticket/schema/ShareTicketService.js index dcfb2bee038..14b4f79202d 100644 --- a/apps/condo/domains/ticket/schema/ShareTicketService.js +++ b/apps/condo/domains/ticket/schema/ShareTicketService.js @@ -11,6 +11,7 @@ const ShareTicketService = new GQLCustomSchema('ShareTicketService', { types: [ { access: true, + // TODO(pahaz): we need to use OrganizationEmployeeWhereUniqueInput and TicketWhereUniqueInput type: 'input ShareTicketInput { sender: SenderFieldInput!, employees: [ID!]!, ticketId: ID! }', }, { diff --git a/apps/condo/domains/ticket/utils/shareDataPacker.spec.js b/apps/condo/domains/ticket/utils/shareDataPacker.spec.js new file mode 100644 index 00000000000..d7621c889ef --- /dev/null +++ b/apps/condo/domains/ticket/utils/shareDataPacker.spec.js @@ -0,0 +1,52 @@ +import { getRandomString } from '@open-condo/keystone/test.utils' + +import { packShareData, unpackShareData } from './shareDataPacker' + +const DATA = '{"date":"2021-06-17T13:26:52.479Z","number":2135,"details":"fragment fragment fragment fragment fragment fragment","id":"3c05ed8c-56b1-41bf-9c9a-8dfeaa07feb1"}' +const V1_ENCRYPTED_DATA = 'gkQ+t4yV53ToCmiGjPcjzzVICyDa76Hy474vWtbUk6P4Am66KU9zqLowFX9C/4kjstxxHbzBTRMqDqrGIjsAasdE9NR2gNLy4hPGATna6pFstDgdY45lPn+XRsuGaL55YHMKTH5gjVrQMbc34+Ro7xEGTiD25d0QngGNe8R4xKZ0JJO9df943k3botuxgyena+bjGZNEvy3vNmA3FPINNA==' +const V2_ENCRYPTED_DATA = '2:pi36FZ28yjeFnSg7hDPgcXIRHjG0jCLETcpR2AUTJtadr21cR/i7YzaiLMf29tjAWcen3ANiQwAfWEcOjRbft9PuEQki2zmnLdDjuMBtO9++4O7S+wy6hfwunBYCMRYk6CyZAWLcdQRNdE8zw6oLxnzVQw==' + +describe('Crypto packShareData and unpackShareData', () => { + it('should correctly pack and unpack data', () => { + const testData = getRandomString() + const packedData = packShareData(testData) + const unpackedData = unpackShareData(packedData) + + expect(unpackedData).toBe(testData) + }) + + it('should produce different encrypted values for different inputs', () => { + const testData1 = 'Hello, world!' + const testData2 = 'Goodbye, world!' + const packedData1 = packShareData(testData1) + const packedData2 = packShareData(testData2) + + expect(packedData1).not.toBe(packedData2) + }) + + it('should throw an error when decrypting invalid data', () => { + const invalidData = 'InvalidEncryptedString' + expect(() => unpackShareData(invalidData)).toThrow() + }) + + it('should produce encrypted text that is different from original text', () => { + const testData = 'Sensitive information' + const packedData = packShareData(testData) + + expect(packedData).not.toBe(testData) + }) + + it('should decrypt existing links', () => { + expect(unpackShareData(V1_ENCRYPTED_DATA)).toEqual(DATA) + }) + + it('should decrypt existing links v2', () => { + expect(unpackShareData(V2_ENCRYPTED_DATA)).toEqual(DATA) + }) + + it('modern method should be more efficient', () => { + const v2 = packShareData(DATA, true) + + expect(v2.length).toBeLessThan(V1_ENCRYPTED_DATA.length) + }) +}) diff --git a/apps/condo/domains/ticket/utils/shareDataPacker.ts b/apps/condo/domains/ticket/utils/shareDataPacker.ts new file mode 100644 index 00000000000..15a281c585c --- /dev/null +++ b/apps/condo/domains/ticket/utils/shareDataPacker.ts @@ -0,0 +1,66 @@ +import crypto from 'crypto' +import zlib from 'zlib' + +const ALGORITHM = 'aes256' +const MODERN_ALGORITHM = 'aes-256-ctr' + +// NOTE: It's a hard-coded credentials by @leonid-d from 2021. +// Good news: This data doesn't contain any secrets ... +// and it's hard to change this key because we have many existing links ... +// nosemgrep: generic.secrets.gitleaks.generic-api-key.generic-api-key +const KEY = '900150983cd24fb0d6963f7d28e17f72' + +const CRYPTOENCODING = 'base64' +const IV_LENGTH = 16 // 16 bytes for AES-CTR +const MODERN_MARKER = '2:' + +export function unpackShareData (data: string): string { + if (data.startsWith(MODERN_MARKER)) { + // New version with marker '2:' using AES-CTR + const trimmedData = data.slice(MODERN_MARKER.length) + const encryptedText = Buffer.from(trimmedData, CRYPTOENCODING) + + // Using KEY directly as IV + const iv = Buffer.from(KEY).subarray(0, IV_LENGTH) + const decipher = crypto.createDecipheriv(MODERN_ALGORITHM, Buffer.from(KEY), iv) + + const decryptedBuffers = [decipher.update(encryptedText), decipher.final()] + // NOTE: it's more efficient but its require modern node version! please use it in a future + // const decompressedText = zlib.brotliDecompressSync(Buffer.concat(decryptedBuffers)).toString('utf8') + const decompressedText = zlib.inflateSync(Buffer.concat(decryptedBuffers)).toString('utf8') + return decompressedText + } else { + // Legacy format (backward compatibility) + // nosemgrep: javascript.node-crypto.security.create-de-cipher-no-iv.create-de-cipher-no-iv + const decipher = crypto.createDecipher(ALGORITHM, KEY) + const decryptedBuffers = [decipher.update(data, CRYPTOENCODING), decipher.final()] + const decryptedText = Buffer.concat(decryptedBuffers).toString('utf8') + return decryptedText + } +} + +// NOTE: we still use old version and will use it until Node.js v23.3.0: https://nodejs.org/api/deprecations.html#DEP0106 +// Probably we want also to use more efficient compression to reduce link size. At the moment I leave old encryption version +export function packShareData (data: string, useModern = false): string { + if (useModern) { + // New version using AES-CTR and Deflate compression + // Using KEY directly as IV + const iv = Buffer.from(KEY).subarray(0, IV_LENGTH) + const cipher = crypto.createCipheriv(MODERN_ALGORITHM, Buffer.from(KEY), iv) + + // NOTE: it's more efficient but its require modern node version! please use it in a future when we will use useModern = true by default! + // const compressedData = zlib.brotliCompressSync(Buffer.from(data), { params: { [zlib.constants.BROTLI_PARAM_QUALITY]: 11 } }) // Use maximum Brotli compression level + const compressedData = zlib.deflateSync(Buffer.from(data)) + const encryptedBuffers = [cipher.update(compressedData), cipher.final()] + + const resultBuffer = Buffer.concat(encryptedBuffers) + return MODERN_MARKER + resultBuffer.toString(CRYPTOENCODING) + } else { + // Legacy format (backward compatibility) + // nosemgrep: javascript.node-crypto.security.create-de-cipher-no-iv.create-de-cipher-no-iv + const cipher = crypto.createCipher(ALGORITHM, KEY) + const encryptedBuffers = [cipher.update(data, 'utf8'), cipher.final()] + const encryptedText = Buffer.concat(encryptedBuffers).toString(CRYPTOENCODING) + return encryptedText + } +} diff --git a/apps/condo/pages/share.tsx b/apps/condo/pages/share.tsx index c39ca599869..d1872a73261 100644 --- a/apps/condo/pages/share.tsx +++ b/apps/condo/pages/share.tsx @@ -1,5 +1,3 @@ -import crypto from 'crypto' - import dayjs from 'dayjs' import get from 'lodash/get' import getConfig from 'next/config' @@ -12,11 +10,7 @@ import { useIntl } from '@open-condo/next/intl' import BaseLayout, { PageWrapper } from '@condo/domains/common/components/containers/BaseLayout' import { LOCALES } from '@condo/domains/common/constants/locale' import { PageComponentType } from '@condo/domains/common/types' -import { - ALGORITHM, - SALT, - CRYPTOENCODING, -} from '@condo/domains/ticket/constants/crypto' +import { unpackShareData } from '@condo/domains/ticket/utils/shareDataPacker' function RedirectToTicket ({ ticketId }) { @@ -80,10 +74,9 @@ const Share: PageComponentType = ({ date, number, details, id }) => } export const getServerSideProps = ({ query }) => { - const decipher = crypto.createDecipher(ALGORITHM, SALT) - const decryptedText = decipher.update(query.q.replace(/\s/gm, '+'), CRYPTOENCODING, 'utf8') + decipher.final('utf8') - // TODO(leonid-d): update encrypt method, or use link shortening service - return { props: JSON.parse(decryptedText) } + const packedData = query.q.replace(/\s/gm, '+') + const shareParams = unpackShareData(packedData) + return { props: JSON.parse(shareParams) } } const EmptyLayout = ({ children, ...props }) => { diff --git a/apps/condorb b/apps/condorb index 2cbbf1f209c..f4adac6f1cb 160000 --- a/apps/condorb +++ b/apps/condorb @@ -1 +1 @@ -Subproject commit 2cbbf1f209c8f0be8a6db6c0fcc8505039efafa5 +Subproject commit f4adac6f1cb4eed881f68916163529e448956c11 diff --git a/apps/telegram-employee-bot b/apps/telegram-employee-bot index 6bcb6f7d07e..e4471e7018d 160000 --- a/apps/telegram-employee-bot +++ b/apps/telegram-employee-bot @@ -1 +1 @@ -Subproject commit 6bcb6f7d07e7984747b514452304aba2ceaeea1b +Subproject commit e4471e7018d0ca9c844ae94e44827eeb6fb2e701 diff --git a/bin/run-semgrep.sh b/bin/run-semgrep.sh index 47c84649b36..ea740855355 100755 --- a/bin/run-semgrep.sh +++ b/bin/run-semgrep.sh @@ -3,8 +3,8 @@ export SEMGREP_RULES="p/default p/expressjs p/react p/nextjs p/sql-injection p/j while getopts "ad:v" flag do case "${flag}" in - a) scan_all=true ;; - d) specified_directory="${OPTARG}" ;; + a) scan_all=true ;; # -a + d) specified_directory="${OPTARG}" ;; # -d esac done