Skip to content

Commit 1ba052c

Browse files
JerT33droctothorpe
authored andcommitted
add valid key checks
Signed-off-by: JerT33 <[email protected]> format code Signed-off-by: JerT33 <[email protected]> change error response to 500 Signed-off-by: JerT33 <[email protected]>
1 parent 19f64ec commit 1ba052c

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

frontend/server/handlers/artifacts.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
findFileOnPodVolume,
2020
parseJSONString,
2121
isAllowedResourceName,
22+
isAllowedObjectKey,
2223
} from '../utils';
2324
import { createMinioClient, getObjectStream } from '../minio-helper';
2425
import * as serverInfo from '../helpers/server-info';
@@ -124,6 +125,10 @@ export function getArtifactsHandler({
124125
res.status(500).send('Storage key is missing from artifact request');
125126
return;
126127
}
128+
if (!isAllowedObjectKey(key)) {
129+
res.status(500).send('Invalid object key');
130+
return;
131+
}
127132
console.log(`Getting storage artifact at: ${source}: ${bucket}/${key}`);
128133

129134
let client: MinioClient;

frontend/server/integration-tests/artifact-get.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -838,6 +838,35 @@ describe('/artifacts', () => {
838838
.get(`/artifacts/get?source=volume&bucket=artifact&key=subartifact/notxist.csv`)
839839
.expect(500, 'Failed to open volume.', done);
840840
});
841+
842+
it('rejects keys with encoded XSS payloads', done => {
843+
const configs = loadConfigs(argv, {
844+
AWS_ACCESS_KEY_ID: 'aws123',
845+
AWS_SECRET_ACCESS_KEY: 'awsSecret123',
846+
});
847+
app = new UIServer(configs);
848+
const request = requests(app.start());
849+
request
850+
.get(
851+
'/artifacts/get?source=s3&namespace=test&peek=256&bucket=ml-pipeline&key=%3Cscript%3Ealert(1)%3C%2Fscript%3E',
852+
)
853+
.expect(500, 'Invalid object key', done);
854+
});
855+
856+
it('rejects keys with repeated unsafe characters', done => {
857+
const configs = loadConfigs(argv, {
858+
AWS_ACCESS_KEY_ID: 'aws123',
859+
AWS_SECRET_ACCESS_KEY: 'awsSecret123',
860+
});
861+
app = new UIServer(configs);
862+
const request = requests(app.start());
863+
request
864+
.get(
865+
'/artifacts/get?source=s3&namespace=test&peek=256&bucket=ml-pipeline&key=' +
866+
'%2b['.repeat(300),
867+
)
868+
.expect(500, 'Invalid object key', done);
869+
});
841870
});
842871

843872
describe('/:source/:bucket/:key', () => {

frontend/server/utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,3 +304,7 @@ function parseK8sError(error: any): ErrorDetails | undefined {
304304
export function isAllowedResourceName(name: string): boolean {
305305
return name.length > 0 && name.length <= 63 && /^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/.test(name);
306306
}
307+
308+
export function isAllowedObjectKey(key: string): boolean {
309+
return key.length > 0 && key.length <= 1024 && /^[a-zA-Z0-9\/\-_.]+$/.test(key);
310+
}

0 commit comments

Comments
 (0)