Skip to content

Commit

Permalink
hotfix(condo): new semgrep rules (#5532)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
pahaz and YEgorLu authored Nov 25, 2024
1 parent 4f4fb78 commit 7463c98
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 22 deletions.
2 changes: 1 addition & 1 deletion apps/callcenter
9 changes: 3 additions & 6 deletions apps/condo/domains/ticket/components/ShareTicketModal.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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`
Expand Down Expand Up @@ -176,14 +174,13 @@ export const ShareTicketModal: React.FC<IShareTicketModalProps> = (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)
Expand Down
3 changes: 3 additions & 0 deletions apps/condo/domains/ticket/constants/crypto.js
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down
1 change: 1 addition & 0 deletions apps/condo/domains/ticket/schema/ShareTicketService.js
Original file line number Diff line number Diff line change
Expand Up @@ -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! }',
},
{
Expand Down
52 changes: 52 additions & 0 deletions apps/condo/domains/ticket/utils/shareDataPacker.spec.js
Original file line number Diff line number Diff line change
@@ -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)
})
})
66 changes: 66 additions & 0 deletions apps/condo/domains/ticket/utils/shareDataPacker.ts
Original file line number Diff line number Diff line change
@@ -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
}
}
15 changes: 4 additions & 11 deletions apps/condo/pages/share.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import crypto from 'crypto'

import dayjs from 'dayjs'
import get from 'lodash/get'
import getConfig from 'next/config'
Expand All @@ -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 }) {
Expand Down Expand Up @@ -80,10 +74,9 @@ const Share: PageComponentType<ShareProps> = ({ 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 }) => {
Expand Down
2 changes: 1 addition & 1 deletion apps/condorb
2 changes: 1 addition & 1 deletion apps/telegram-employee-bot
4 changes: 2 additions & 2 deletions bin/run-semgrep.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 <directory>
esac
done

Expand Down

0 comments on commit 7463c98

Please sign in to comment.