Skip to content

Commit

Permalink
Merge pull request #336 from Workable/PROD-30208_fix-porssible-ssrf-a…
Browse files Browse the repository at this point in the history
…ttack-on-s3-url

[PROD-30208] - add check on s3 url for @ after domain to prevent security risks
  • Loading branch information
CharisVrettos authored Sep 21, 2023
2 parents e830958 + 681add6 commit 85fcb4b
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/initializers/joi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ const Joi: JoiWithExtensions = _Joi.extend(
}
],
validate(value, helpers, {bucket}) {
if (!isOwnS3Path(bucket, value)) {
const { hostname, protocol, pathname} = new URL(value);
if (!isOwnS3Path(bucket, `${protocol}//${hostname}${pathname}`)) {
return helpers.error('string.notInS3');
}
return value;
Expand Down
16 changes: 15 additions & 1 deletion test/initializers/joi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,22 @@ describe('joi extensions', function () {
should(underTest.error).be.undefined();
underTest.value.should.equal(url);
});
it('s3 url with @ after domain should throw error', function () {
const urls = [
'https://[email protected]/tmp/123-456-789',
'https://[email protected]/tmp/123-456-789',
'https://application-form.s3.amazonaws.com@joecup/tmp/123-456-789',
'https://application-form.s3.amazonaws.com@hello',
'https://application-form.s3.amazonaws.com@4nsex02yymx4wzjzc0ljcmsar1xsli97.oastify.com'
];
urls.forEach(url => {
const underTest = Joi.urlInOwnS3()
.bucket('application-form')
.validate(url);
underTest.error.message.should.equal('Invalid path provided');
});
});
});

describe('url', function () {
it('rejects URLs with less than two domain fragments', function () {
Joi.url().validate('https://foo').error.message.should.equal('"value" must contain a valid domain name');
Expand Down

0 comments on commit 85fcb4b

Please sign in to comment.