Skip to content

Commit dbbb8fe

Browse files
yakanechiyakanechi
authored
fix: Verify signature of payloads containing control characters [2] (#250)
1 parent 4d0c98e commit dbbb8fe

File tree

2 files changed

+92
-66
lines changed

2 files changed

+92
-66
lines changed

‎index.js‎

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,24 +1594,30 @@ class GithubScm extends Scm {
15941594
* @async _parseHook
15951595
* @param {Object} payloadHeaders The request headers associated with the
15961596
* webhook payload
1597-
* @param {Object} webhookPayload The webhook payload received from the
1597+
* @param {String} webhookPayload The webhook payload received from the
15981598
* SCM service.
15991599
* @return {Promise} A key-map of data related to the received
16001600
* payload
16011601
*/
16021602
async _parseHook(payloadHeaders, webhookPayload) {
16031603
const signature = payloadHeaders['x-hub-signature'];
1604-
16051604
const type = payloadHeaders['x-github-event'];
16061605
const hookId = payloadHeaders['x-github-delivery'];
1607-
const checkoutUrl = hoek.reach(webhookPayload, 'repository.ssh_url');
16081606
const scmContexts = this._getScmContexts();
16091607
const scmContext = scmContexts[0];
16101608
const commitAuthors = [];
1611-
const commits = hoek.reach(webhookPayload, 'commits');
1612-
const deleted = hoek.reach(webhookPayload, 'deleted');
16131609

16141610
const checkoutSshHost = this.config.gheHost ? this.config.gheHost : 'github.com';
1611+
1612+
// eslint-disable-next-line no-underscore-dangle
1613+
if (!(await verify(this.config.secret, webhookPayload, signature))) {
1614+
throwError('Invalid x-hub-signature', 400);
1615+
}
1616+
1617+
const parsedWebhookPayload = JSON.parse(webhookPayload);
1618+
const checkoutUrl = hoek.reach(parsedWebhookPayload, 'repository.ssh_url');
1619+
const commits = hoek.reach(parsedWebhookPayload, 'commits');
1620+
const deleted = hoek.reach(parsedWebhookPayload, 'deleted');
16151621
const regexMatchArray = checkoutUrl.match(CHECKOUT_URL_REGEX);
16161622

16171623
if (!regexMatchArray || regexMatchArray[1] !== checkoutSshHost) {
@@ -1620,26 +1626,21 @@ class GithubScm extends Scm {
16201626

16211627
// additional check for github enterprise cloud hooks
16221628
if (this.config.gheCloud) {
1623-
const enterpriseSlug = hoek.reach(webhookPayload, 'enterprise.slug');
1629+
const enterpriseSlug = hoek.reach(parsedWebhookPayload, 'enterprise.slug');
16241630

16251631
if (this.config.gheCloudSlug !== enterpriseSlug) {
16261632
throwError(`Skipping incorrect scm context for hook parsing, ${checkoutUrl}, ${scmContext}`, 400);
16271633
}
16281634
}
16291635

1630-
// eslint-disable-next-line no-underscore-dangle
1631-
if (!(await verify(this.config.secret, JSON.stringify(webhookPayload), signature))) {
1632-
throwError('Invalid x-hub-signature', 400);
1633-
}
1634-
16351636
switch (type) {
16361637
case 'pull_request': {
1637-
let action = hoek.reach(webhookPayload, 'action');
1638-
const prNum = hoek.reach(webhookPayload, 'pull_request.number');
1639-
const prTitle = hoek.reach(webhookPayload, 'pull_request.title');
1640-
const baseSource = hoek.reach(webhookPayload, 'pull_request.base.repo.id');
1641-
const headSource = hoek.reach(webhookPayload, 'pull_request.head.repo.id');
1642-
const prMerged = hoek.reach(webhookPayload, 'pull_request.merged');
1638+
let action = hoek.reach(parsedWebhookPayload, 'action');
1639+
const prNum = hoek.reach(parsedWebhookPayload, 'pull_request.number');
1640+
const prTitle = hoek.reach(parsedWebhookPayload, 'pull_request.title');
1641+
const baseSource = hoek.reach(parsedWebhookPayload, 'pull_request.base.repo.id');
1642+
const headSource = hoek.reach(parsedWebhookPayload, 'pull_request.head.repo.id');
1643+
const prMerged = hoek.reach(parsedWebhookPayload, 'pull_request.merged');
16431644
const prSource = baseSource === headSource ? 'branch' : 'fork';
16441645
const ref = `pull/${prNum}/merge`;
16451646

@@ -1656,23 +1657,23 @@ class GithubScm extends Scm {
16561657

16571658
return {
16581659
action,
1659-
branch: hoek.reach(webhookPayload, 'pull_request.base.ref'),
1660+
branch: hoek.reach(parsedWebhookPayload, 'pull_request.base.ref'),
16601661
checkoutUrl,
16611662
prNum,
16621663
prTitle,
16631664
prRef: ref,
16641665
ref,
16651666
prSource,
1666-
sha: hoek.reach(webhookPayload, 'pull_request.head.sha'),
1667+
sha: hoek.reach(parsedWebhookPayload, 'pull_request.head.sha'),
16671668
type: 'pr',
1668-
username: hoek.reach(webhookPayload, 'sender.login'),
1669+
username: hoek.reach(parsedWebhookPayload, 'sender.login'),
16691670
hookId,
16701671
scmContext,
16711672
prMerged
16721673
};
16731674
}
16741675
case 'push': {
1675-
const ref = hoek.reach(webhookPayload, 'ref');
1676+
const ref = hoek.reach(parsedWebhookPayload, 'ref');
16761677

16771678
// repository tag pushed
16781679
if (ref.startsWith('refs/tags/')) {
@@ -1691,44 +1692,44 @@ class GithubScm extends Scm {
16911692

16921693
return {
16931694
action: 'push',
1694-
branch: hoek.reach(webhookPayload, 'ref').replace(/^refs\/heads\//, ''),
1695+
branch: hoek.reach(parsedWebhookPayload, 'ref').replace(/^refs\/heads\//, ''),
16951696
checkoutUrl,
1696-
sha: hoek.reach(webhookPayload, 'after'),
1697+
sha: hoek.reach(parsedWebhookPayload, 'after'),
16971698
type: 'repo',
1698-
username: hoek.reach(webhookPayload, 'sender.login'),
1699+
username: hoek.reach(parsedWebhookPayload, 'sender.login'),
16991700
commitAuthors,
1700-
lastCommitMessage: hoek.reach(webhookPayload, 'head_commit.message') || '',
1701+
lastCommitMessage: hoek.reach(parsedWebhookPayload, 'head_commit.message') || '',
17011702
hookId,
17021703
scmContext,
1703-
ref: hoek.reach(webhookPayload, 'ref'),
1704-
addedFiles: hoek.reach(webhookPayload, 'head_commit.added', { default: [] }),
1705-
modifiedFiles: hoek.reach(webhookPayload, 'head_commit.modified', { default: [] }),
1706-
removedFiles: hoek.reach(webhookPayload, 'head_commit.removed', { default: [] })
1704+
ref: hoek.reach(parsedWebhookPayload, 'ref'),
1705+
addedFiles: hoek.reach(parsedWebhookPayload, 'head_commit.added', { default: [] }),
1706+
modifiedFiles: hoek.reach(parsedWebhookPayload, 'head_commit.modified', { default: [] }),
1707+
removedFiles: hoek.reach(parsedWebhookPayload, 'head_commit.removed', { default: [] })
17071708
};
17081709
}
17091710
case 'release': {
1710-
const action = hoek.reach(webhookPayload, 'action');
1711+
const action = hoek.reach(parsedWebhookPayload, 'action');
17111712

17121713
if (!PERMITTED_RELEASE_EVENT.includes(action)) {
17131714
return null;
17141715
}
17151716

17161717
return {
17171718
action: 'release',
1718-
branch: hoek.reach(webhookPayload, 'repository.default_branch'),
1719+
branch: hoek.reach(parsedWebhookPayload, 'repository.default_branch'),
17191720
checkoutUrl,
17201721
type: 'repo',
1721-
username: hoek.reach(webhookPayload, 'sender.login'),
1722+
username: hoek.reach(parsedWebhookPayload, 'sender.login'),
17221723
hookId,
17231724
scmContext,
1724-
ref: hoek.reach(webhookPayload, 'release.tag_name'),
1725-
releaseId: hoek.reach(webhookPayload, 'release.id').toString(),
1726-
releaseName: hoek.reach(webhookPayload, 'release.name') || '',
1727-
releaseAuthor: hoek.reach(webhookPayload, 'release.author.login') || ''
1725+
ref: hoek.reach(parsedWebhookPayload, 'release.tag_name'),
1726+
releaseId: hoek.reach(parsedWebhookPayload, 'release.id').toString(),
1727+
releaseName: hoek.reach(parsedWebhookPayload, 'release.name') || '',
1728+
releaseAuthor: hoek.reach(parsedWebhookPayload, 'release.author.login') || ''
17281729
};
17291730
}
17301731
case 'create': {
1731-
const refType = hoek.reach(webhookPayload, 'ref_type');
1732+
const refType = hoek.reach(parsedWebhookPayload, 'ref_type');
17321733

17331734
if (refType !== 'tag') {
17341735
logger.info('%s event of %s is not available yet in scm-github plugin', type, refType);
@@ -1738,13 +1739,13 @@ class GithubScm extends Scm {
17381739

17391740
return {
17401741
action: 'tag',
1741-
branch: hoek.reach(webhookPayload, 'repository.default_branch'),
1742+
branch: hoek.reach(parsedWebhookPayload, 'repository.default_branch'),
17421743
checkoutUrl,
17431744
type: 'repo',
1744-
username: hoek.reach(webhookPayload, 'sender.login'),
1745+
username: hoek.reach(parsedWebhookPayload, 'sender.login'),
17451746
hookId,
17461747
scmContext,
1747-
ref: hoek.reach(webhookPayload, 'ref')
1748+
ref: hoek.reach(parsedWebhookPayload, 'ref')
17481749
};
17491750
}
17501751

@@ -1986,7 +1987,7 @@ class GithubScm extends Scm {
19861987
* Determine if an scm module can handle the received webhook
19871988
* @async _canHandleWebhook
19881989
* @param {Object} headers The request headers associated with the webhook payload
1989-
* @param {Object} payload The webhook payload received from the SCM service
1990+
* @param {String} payload The webhook payload received from the SCM service
19901991
* @return {Promise} Resolves a boolean denoting whether scm module supports webhook
19911992
*/
19921993
async _canHandleWebhook(headers, payload) {

0 commit comments

Comments
 (0)