From 152c3808b045d4b177bf1f409c043965660f4fe5 Mon Sep 17 00:00:00 2001 From: ChaitanyaD48 Date: Mon, 11 Nov 2024 00:52:18 +0530 Subject: [PATCH 1/3] feat: create checkCryptoImplementation file for detecting non-standard cryptography Signed-off-by: ChaitanyaD48 --- .../push-action/checkCryptoImplementation.js | 140 ++++++++++++++++++ src/proxy/processors/push-action/index.js | 1 + 2 files changed, 141 insertions(+) create mode 100644 src/proxy/processors/push-action/checkCryptoImplementation.js diff --git a/src/proxy/processors/push-action/checkCryptoImplementation.js b/src/proxy/processors/push-action/checkCryptoImplementation.js new file mode 100644 index 000000000..5be14bf30 --- /dev/null +++ b/src/proxy/processors/push-action/checkCryptoImplementation.js @@ -0,0 +1,140 @@ +const Step = require('../../actions').Step; + +// Common encryption-related patterns and keywords +const CRYPTO_PATTERNS = { + // Known non-standard encryption algorithms + nonStandardAlgorithms: [ + 'xor\\s*\\(', + 'rot13', + 'caesar\\s*cipher', + 'custom\\s*encrypt', + 'simple\\s*encrypt', + 'homebrew\\s*crypto', + 'custom\\s*hash' + ], + + // Suspicious operations that might indicate custom crypto Implementation + suspiciousOperations: [ + 'bit\\s*shift', + 'bit\\s*rotate', + '\\^=', + '\\^', + '>>>', + '<<<', + 'shuffle\\s*bytes' + ], + + // Common encryption-related variable names + suspiciousVariables: [ + 'cipher', + 'encrypt', + 'decrypt', + 'scramble', + 'salt(?!\\w)', + 'iv(?!\\w)', + 'nonce' + ] +}; + +function analyzeCodeForCrypto(diffContent) { + const issues = []; + // Check for above mentioned cryto Patterns + if(!diffContent) return issues; + + CRYPTO_PATTERNS.nonStandardAlgorithms.forEach(pattern => { + const regex = new RegExp(pattern, 'gi'); + const matches = diffContent.match(regex); + if (matches) { + issues.push({ + type: 'non_standard_algorithm', + pattern: pattern, + matches: matches, + severity: 'high', + message: `Detected possible non-standard encryption algorithm: ${matches.join(', ')}` + }); + } + }); + + CRYPTO_PATTERNS.suspiciousOperations.forEach(pattern => { + const regex = new RegExp(pattern, 'gi'); + const matches = diffContent.match(regex); + if (matches) { + issues.push({ + type: 'suspicious_operation', + pattern: pattern, + matches: matches, + severity: 'medium', + message: `Detected suspicious cryptographic operation: ${matches.join(', ')}` + }); + } + }); + + CRYPTO_PATTERNS.suspiciousVariables.forEach(pattern => { + const regex = new RegExp(pattern, 'gi'); + const matches = diffContent.match(regex); + if (matches) { + issues.push({ + type: 'suspicious_variable', + pattern: pattern, + matches: matches, + severity: 'low', + message: `Detected potential encryption-related variable: ${matches.join(', ')}` + }); + } + }); + + return issues; +} + +const exec = async (req, action) => { + const step = new Step('checkCryptoImplementation'); + + try { + let hasIssues = false; + const allIssues = []; + + for (const commit of action.commitData) { + const diff = commit.diff || ''; + const issues = analyzeCodeForCrypto(diff); + + if (issues.length > 0) { + hasIssues = true; + allIssues.push({ + commit: commit.hash, + issues: issues + }); + } + } + + if (hasIssues) { + step.error = true; + + const errorMessage = allIssues.map(commitIssues => { + return `Commit ${commitIssues.commit}:\n` + + commitIssues.issues.map(issue => + `- ${issue.severity.toUpperCase()}: ${issue.message}` + ).join('\n'); + }).join('\n\n'); + + step.setError( + '\n\nYour push has been blocked.\n' + + 'Potential non-standard cryptographic implementations detected:\n\n' + + `${errorMessage}\n\n` + + 'Please use standard cryptographic libraries instead of custom implementations.\n' + + 'Recommended: Use established libraries like crypto, node-forge, or Web Crypto API.\n' + ); + } + + action.addStep(step); + return action; + } catch (error) { + step.error = true; + step.setError(`Error analyzing crypto implementation: ${error.message}`); + action.addStep(step); + return action; + } +}; + +// exec.displayName = 'checkCryptoImplementation.exec'; +exports.exec = exec; +exports.analyzeCodeForCrypto = analyzeCodeForCrypto; \ No newline at end of file diff --git a/src/proxy/processors/push-action/index.js b/src/proxy/processors/push-action/index.js index 72a97b33c..2832ae42a 100644 --- a/src/proxy/processors/push-action/index.js +++ b/src/proxy/processors/push-action/index.js @@ -11,3 +11,4 @@ exports.checkCommitMessages = require('./checkCommitMessages').exec; exports.checkAuthorEmails = require('./checkAuthorEmails').exec; exports.checkUserPushPermission = require('./checkUserPushPermission').exec; exports.clearBareClone = require('./clearBareClone').exec; +exports.checkCryptoImplementation = require('./checkCryptoImplementation').exec; \ No newline at end of file From 3c929d57fa5d9f63d3bb31ce0ad4f0247c6ea5ab Mon Sep 17 00:00:00 2001 From: ChaitanyaD48 Date: Mon, 11 Nov 2024 00:52:49 +0530 Subject: [PATCH 2/3] test: add test cases for checkCryptoImplementation Signed-off-by: ChaitanyaD48 --- test/checkCryptoImplementation.test.js | 223 +++++++++++++++++++++++++ 1 file changed, 223 insertions(+) create mode 100644 test/checkCryptoImplementation.test.js diff --git a/test/checkCryptoImplementation.test.js b/test/checkCryptoImplementation.test.js new file mode 100644 index 000000000..1d181c572 --- /dev/null +++ b/test/checkCryptoImplementation.test.js @@ -0,0 +1,223 @@ +const { expect } = require('chai'); +const { analyzeCodeForCrypto, exec } = require('../src/proxy/processors/push-action/checkCryptoImplementation.js'); + +describe('Crypto Implementation Check Plugin', () => { + describe('analyzeCodeForCrypto', () => { + it('should detect non-standard encryption algorithms', () => { + const testCode = ` + function customEncrypt(data) { + return data.split('').map(char => + String.fromCharCode(char.charCodeAt(0) ^ 0x7F) + ).join(''); + } + `; + + const issues = analyzeCodeForCrypto(testCode); + expect(issues).to.have.lengthOf.at.least(1); + expect(issues.some(i => i.type === 'non_standard_algorithm')).to.be.true; + }); + + it('should detect suspicious bit operations', () => { + const testCode = ` + function scrambleData(data) { + let result = ''; + for(let i = 0; i < data.length; i++) { + result += String.fromCharCode(data.charCodeAt(i) >>> 2); + } + return result; + } + `; + + const issues = analyzeCodeForCrypto(testCode); + expect(issues).to.have.lengthOf.at.least(1); + expect(issues.some(i => i.type === 'suspicious_operation')).to.be.true; + }); + + it('should detect suspicious variable names', () => { + const testCode = ` + const cipher = {}; + let salt = generateRandomBytes(16); + const iv = new Uint8Array(12); + `; + + const issues = analyzeCodeForCrypto(testCode); + expect(issues).to.have.lengthOf.at.least(3); + expect(issues.some(i => i.type === 'suspicious_variable')).to.be.true; + }); + + it('should not flag standard crypto library usage', () => { + const testCode = ` + const crypto = require('crypto'); + const cipher = crypto.createCipheriv('aes-256-gcm', key, iv); + `; + + const issues = analyzeCodeForCrypto(testCode); + expect(issues.filter(i => i.severity === 'high')).to.have.lengthOf(0); + }); + + it('should handle empty input', () => { + const issues = analyzeCodeForCrypto(''); + expect(issues).to.be.an('array').that.is.empty; + }); + + it('should handle null or undefined input', () => { + expect(analyzeCodeForCrypto(null)).to.be.an('array').that.is.empty; + expect(analyzeCodeForCrypto(undefined)).to.be.an('array').that.is.empty; + }); + + }); + + describe('exec', () => { + + it('should handle empty diff content', async () => { + const req = {}; + const action = { + commitData: [{ + hash: '123abc', + diff: '' + }], + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step.error).to.be.false; + }); + + it('should handle undefined diff content', async () => { + const req = {}; + const action = { + commitData: [{ + hash: '123abc' + // diff is undefined + }], + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step.error).to.be.false; + }); + + it('should handle empty commitData array', async () => { + const req = {}; + const action = { + commitData: [], + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step.error).to.be.false; + }); + it('should block commits with non-standard crypto implementations', async () => { + const req = {}; + const action = { + commitData: [{ + hash: '123abc', + diff: ` + function customEncrypt(data) { + return data.split('').map(char => + String.fromCharCode(char.charCodeAt(0) ^ 0x7F) + ).join(''); + } + ` + }], + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step.error).to.be.true; + }); + + it('should allow commits without crypto issues', async () => { + const req = {}; + const action = { + commitData: [{ + hash: '123abc', + diff: ` + function normalFunction() { + return 'Hello World'; + } + ` + }], + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step.error).to.be.false; + }); + + it('should handle multiple commits', async () => { + const req = {}; + const action = { + commitData: [ + { + hash: '123abc', + diff: `function safe() { return true; }` + }, + { + hash: '456def', + diff: ` + function rot13(str) { + return str.replace(/[a-zA-Z]/g, c => + String.fromCharCode((c <= 'Z' ? 90 : 122) >= (c = c.charCodeAt(0) + 13) ? c : c - 26) + ); + } + ` + } + ], + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step).to.have.property('error', true); +}); + + + it('should handle errors gracefully', async () => { + const req = {}; + const action = { + commitData: null, + addStep: function(step) { this.step = step; } + }; + + const result = await exec(req, action); + expect(result.step.error).to.be.true; + }); + }); + + describe('Pattern Detection', () => { + it('should detect various forms of XOR encryption', () => { + const testCases = [ + `function encrypt(a, b) { return a ^= b; }`, + `const result = data ^ key;`, + `function xor(plaintext, key) { return plaintext ^ key; }`, + `return char ^ 0xFF;` + ]; + + testCases.forEach(testCode => { + const issues = analyzeCodeForCrypto(testCode); + const hasXORIssue = issues.some(issue => + issue.type === 'suspicious_operation' || + issue.message.toLowerCase().includes('xor') + ); + expect(hasXORIssue, `Failed to detect XOR in: ${testCode}`).to.be.true; + }); + }); + + it('should detect custom hash implementations', () => { + const testCode = ` + function customHash(input) { + let hash = 0; + for(let i = 0; i < input.length; i++) { + hash = ((hash << 5) - hash) + input.charCodeAt(i); + hash = hash & hash; + } + return hash; + } + `; + + const issues = analyzeCodeForCrypto(testCode); + expect(issues).to.have.lengthOf.at.least(1); + expect(issues.some(i => i.severity === 'high')).to.be.true; + }); + }); +}); \ No newline at end of file From 7cf24ee441b465e60ab6fd508bd1830b53f02c26 Mon Sep 17 00:00:00 2001 From: ChaitanyaD48 Date: Mon, 11 Nov 2024 00:53:17 +0530 Subject: [PATCH 3/3] feat: integrate checkCryptoImplementation into the main processing chain Signed-off-by: ChaitanyaD48 --- src/proxy/chain.js | 1 + test/chain.test.js | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/src/proxy/chain.js b/src/proxy/chain.js index 11e6ae106..a7ba7203e 100644 --- a/src/proxy/chain.js +++ b/src/proxy/chain.js @@ -11,6 +11,7 @@ const pushActionChain = [ proc.push.writePack, proc.push.getDiff, proc.push.clearBareClone, + proc.push.checkCryptoImplementation, proc.push.scanDiff, proc.push.blockForAuth, ]; diff --git a/test/chain.test.js b/test/chain.test.js index 33d5750ac..cc5f5ab6d 100644 --- a/test/chain.test.js +++ b/test/chain.test.js @@ -19,6 +19,7 @@ const mockPushProcessors = { audit: sinon.stub(), checkRepoInAuthorisedList: sinon.stub(), checkCommitMessages: sinon.stub(), + checkCryptoImplementation: sinon.stub(), checkAuthorEmails: sinon.stub(), checkUserPushPermission: sinon.stub(), checkIfWaitingAuth: sinon.stub(), @@ -33,6 +34,7 @@ mockPushProcessors.parsePush.displayName = 'parsePush'; mockPushProcessors.audit.displayName = 'audit'; mockPushProcessors.checkRepoInAuthorisedList.displayName = 'checkRepoInAuthorisedList'; mockPushProcessors.checkCommitMessages.displayName = 'checkCommitMessages'; +mockPushProcessors.checkCryptoImplementation.displayName = 'checkCryptoImplementation'; mockPushProcessors.checkAuthorEmails.displayName = 'checkAuthorEmails'; mockPushProcessors.checkUserPushPermission.displayName = 'checkUserPushPermission'; mockPushProcessors.checkIfWaitingAuth.displayName = 'checkIfWaitingAuth'; @@ -106,6 +108,7 @@ describe('proxy chain', function () { mockPushProcessors.checkCommitMessages.resolves(continuingAction); mockPushProcessors.checkAuthorEmails.resolves(continuingAction); mockPushProcessors.checkUserPushPermission.resolves(continuingAction); + mockPushProcessors.checkCryptoImplementation.resolves(continuingAction); // this stops the chain from further execution mockPushProcessors.checkIfWaitingAuth.resolves({ type: 'push', continue: () => false, allowPush: false }); @@ -120,6 +123,7 @@ describe('proxy chain', function () { expect(mockPushProcessors.checkIfWaitingAuth.called).to.be.true; expect(mockPushProcessors.pullRemote.called).to.be.false; expect(mockPushProcessors.audit.called).to.be.true; + expect(mockPushProcessors.checkCryptoImplementation.called).to.be.true; expect(result.type).to.equal('push'); expect(result.allowPush).to.be.false; @@ -131,10 +135,12 @@ describe('proxy chain', function () { const continuingAction = { type: 'push', continue: () => true, allowPush: false }; mockPreProcessors.parseAction.resolves({ type: 'push' }); mockPushProcessors.parsePush.resolves(continuingAction); + mockPushProcessors.checkCryptoImplementation.resolves(continuingAction); mockPushProcessors.checkRepoInAuthorisedList.resolves(continuingAction); mockPushProcessors.checkCommitMessages.resolves(continuingAction); mockPushProcessors.checkAuthorEmails.resolves(continuingAction); mockPushProcessors.checkUserPushPermission.resolves(continuingAction); + // this stops the chain from further execution mockPushProcessors.checkIfWaitingAuth.resolves({ type: 'push', continue: () => true, allowPush: true }); const result = await chain.executeChain(req); @@ -148,6 +154,7 @@ describe('proxy chain', function () { expect(mockPushProcessors.checkIfWaitingAuth.called).to.be.true; expect(mockPushProcessors.pullRemote.called).to.be.false; expect(mockPushProcessors.audit.called).to.be.true; + expect(mockPushProcessors.checkCryptoImplementation.called).to.be.true; expect(result.type).to.equal('push'); expect(result.allowPush).to.be.true; @@ -170,6 +177,7 @@ describe('proxy chain', function () { mockPushProcessors.clearBareClone.resolves(continuingAction); mockPushProcessors.scanDiff.resolves(continuingAction); mockPushProcessors.blockForAuth.resolves(continuingAction); + mockPushProcessors.checkCryptoImplementation.resolves(continuingAction); const result = await chain.executeChain(req); @@ -187,6 +195,7 @@ describe('proxy chain', function () { expect(mockPushProcessors.scanDiff.called).to.be.true; expect(mockPushProcessors.blockForAuth.called).to.be.true; expect(mockPushProcessors.audit.called).to.be.true; + expect(mockPushProcessors.checkCryptoImplementation.called).to.be.true; expect(result.type).to.equal('push'); expect(result.allowPush).to.be.false;