Skip to content

Commit 3d7a61e

Browse files
mtrezzamtrezza
authored
fix: server crashes when receiving file download request with invalid byte range; this fixes a security vulnerability that allows an attacker to impact the availability of the server instance; the fix improves parsing of the range parameter to properly handle invalid range requests ([GHSA-h423-w6qv-2wj3](GHSA-h423-w6qv-2wj3)) (#8236)
1 parent f03bf00 commit 3d7a61e

File tree

3 files changed

+228
-21
lines changed

3 files changed

+228
-21
lines changed

‎spec/ParseFile.spec.js

Lines changed: 199 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,198 @@ describe('Parse.File testing', () => {
661661
});
662662
});
663663

664-
xdescribe('Gridstore Range tests', () => {
664+
describe_only_db('mongo')('Gridstore Range', () => {
665+
it('supports bytes range out of range', async () => {
666+
const headers = {
667+
'Content-Type': 'application/octet-stream',
668+
'X-Parse-Application-Id': 'test',
669+
'X-Parse-REST-API-Key': 'rest',
670+
};
671+
const response = await request({
672+
method: 'POST',
673+
headers: headers,
674+
url: 'http://localhost:8378/1//files/file.txt ',
675+
body: repeat('argle bargle', 100),
676+
});
677+
const b = response.data;
678+
const file = await request({
679+
url: b.url,
680+
headers: {
681+
'Content-Type': 'application/octet-stream',
682+
'X-Parse-Application-Id': 'test',
683+
Range: 'bytes=15000-18000',
684+
},
685+
}).catch(e => e);
686+
expect(file.headers['content-range']).toBe('bytes 1212-1212/1212');
687+
});
688+
689+
it('supports bytes range if end greater than start', async () => {
690+
const headers = {
691+
'Content-Type': 'application/octet-stream',
692+
'X-Parse-Application-Id': 'test',
693+
'X-Parse-REST-API-Key': 'rest',
694+
};
695+
const response = await request({
696+
method: 'POST',
697+
headers: headers,
698+
url: 'http://localhost:8378/1//files/file.txt ',
699+
body: repeat('argle bargle', 100),
700+
});
701+
const b = response.data;
702+
const file = await request({
703+
url: b.url,
704+
headers: {
705+
'Content-Type': 'application/octet-stream',
706+
'X-Parse-Application-Id': 'test',
707+
Range: 'bytes=15000-100',
708+
},
709+
});
710+
expect(file.headers['content-range']).toBe('bytes 100-1212/1212');
711+
});
712+
713+
it('supports bytes range if end is undefined', async () => {
714+
const headers = {
715+
'Content-Type': 'application/octet-stream',
716+
'X-Parse-Application-Id': 'test',
717+
'X-Parse-REST-API-Key': 'rest',
718+
};
719+
const response = await request({
720+
method: 'POST',
721+
headers: headers,
722+
url: 'http://localhost:8378/1//files/file.txt ',
723+
body: repeat('argle bargle', 100),
724+
});
725+
const b = response.data;
726+
const file = await request({
727+
url: b.url,
728+
headers: {
729+
'Content-Type': 'application/octet-stream',
730+
'X-Parse-Application-Id': 'test',
731+
Range: 'bytes=100-',
732+
},
733+
});
734+
expect(file.headers['content-range']).toBe('bytes 100-1212/1212');
735+
});
736+
737+
it('supports bytes range if start and end undefined', async () => {
738+
const headers = {
739+
'Content-Type': 'application/octet-stream',
740+
'X-Parse-Application-Id': 'test',
741+
'X-Parse-REST-API-Key': 'rest',
742+
};
743+
const response = await request({
744+
method: 'POST',
745+
headers: headers,
746+
url: 'http://localhost:8378/1//files/file.txt ',
747+
body: repeat('argle bargle', 100),
748+
});
749+
const b = response.data;
750+
const file = await request({
751+
url: b.url,
752+
headers: {
753+
'Content-Type': 'application/octet-stream',
754+
'X-Parse-Application-Id': 'test',
755+
Range: 'bytes=abc-efs',
756+
},
757+
}).catch(e => e);
758+
expect(file.headers['content-range']).toBeUndefined();
759+
});
760+
761+
it('supports bytes range if start and end undefined', async () => {
762+
const headers = {
763+
'Content-Type': 'application/octet-stream',
764+
'X-Parse-Application-Id': 'test',
765+
'X-Parse-REST-API-Key': 'rest',
766+
};
767+
const response = await request({
768+
method: 'POST',
769+
headers: headers,
770+
url: 'http://localhost:8378/1//files/file.txt ',
771+
body: repeat('argle bargle', 100),
772+
});
773+
const b = response.data;
774+
const file = await request({
775+
url: b.url,
776+
headers: {
777+
'Content-Type': 'application/octet-stream',
778+
'X-Parse-Application-Id': 'test',
779+
},
780+
}).catch(e => e);
781+
expect(file.headers['content-range']).toBeUndefined();
782+
});
783+
784+
it('supports bytes range if end is greater than size', async () => {
785+
const headers = {
786+
'Content-Type': 'application/octet-stream',
787+
'X-Parse-Application-Id': 'test',
788+
'X-Parse-REST-API-Key': 'rest',
789+
};
790+
const response = await request({
791+
method: 'POST',
792+
headers: headers,
793+
url: 'http://localhost:8378/1//files/file.txt ',
794+
body: repeat('argle bargle', 100),
795+
});
796+
const b = response.data;
797+
const file = await request({
798+
url: b.url,
799+
headers: {
800+
'Content-Type': 'application/octet-stream',
801+
'X-Parse-Application-Id': 'test',
802+
Range: 'bytes=0-2000',
803+
},
804+
}).catch(e => e);
805+
expect(file.headers['content-range']).toBe('bytes 0-1212/1212');
806+
});
807+
808+
it('supports bytes range if end is greater than size', async () => {
809+
const headers = {
810+
'Content-Type': 'application/octet-stream',
811+
'X-Parse-Application-Id': 'test',
812+
'X-Parse-REST-API-Key': 'rest',
813+
};
814+
const response = await request({
815+
method: 'POST',
816+
headers: headers,
817+
url: 'http://localhost:8378/1//files/file.txt ',
818+
body: repeat('argle bargle', 100),
819+
});
820+
const b = response.data;
821+
const file = await request({
822+
url: b.url,
823+
headers: {
824+
'Content-Type': 'application/octet-stream',
825+
'X-Parse-Application-Id': 'test',
826+
Range: 'bytes=0-2000',
827+
},
828+
}).catch(e => e);
829+
expect(file.headers['content-range']).toBe('bytes 0-1212/1212');
830+
});
831+
832+
it('supports bytes range with 0 length', async () => {
833+
const headers = {
834+
'Content-Type': 'application/octet-stream',
835+
'X-Parse-Application-Id': 'test',
836+
'X-Parse-REST-API-Key': 'rest',
837+
};
838+
const response = await request({
839+
method: 'POST',
840+
headers: headers,
841+
url: 'http://localhost:8378/1//files/file.txt ',
842+
body: 'a',
843+
}).catch(e => e);
844+
const b = response.data;
845+
const file = await request({
846+
url: b.url,
847+
headers: {
848+
'Content-Type': 'application/octet-stream',
849+
'X-Parse-Application-Id': 'test',
850+
Range: 'bytes=-2000',
851+
},
852+
}).catch(e => e);
853+
expect(file.headers['content-range']).toBe('bytes 0-1/1');
854+
});
855+
665856
it('supports range requests', done => {
666857
const headers = {
667858
'Content-Type': 'application/octet-stream',
@@ -750,7 +941,7 @@ describe('Parse.File testing', () => {
750941
});
751942
});
752943

753-
xit('supports getting last n bytes', done => {
944+
it('supports getting last n bytes', done => {
754945
const headers = {
755946
'Content-Type': 'application/octet-stream',
756947
'X-Parse-Application-Id': 'test',
@@ -848,21 +1039,19 @@ describe('Parse.File testing', () => {
8481039
});
8491040
});
8501041

851-
it('fails to stream unknown file', done => {
852-
request({
1042+
it('fails to stream unknown file', async () => {
1043+
const response = await request({
8531044
url: 'http://localhost:8378/1/files/test/file.txt',
8541045
headers: {
8551046
'Content-Type': 'application/octet-stream',
8561047
'X-Parse-Application-Id': 'test',
8571048
'X-Parse-REST-API-Key': 'rest',
8581049
Range: 'bytes=13-240',
8591050
},
860-
}).then(response => {
861-
expect(response.status).toBe(404);
862-
const body = response.text;
863-
expect(body).toEqual('File not found.');
864-
done();
865-
});
1051+
}).catch(e => e);
1052+
expect(response.status).toBe(404);
1053+
const body = response.text;
1054+
expect(body).toEqual('File not found.');
8661055
});
8671056
});
8681057

‎src/Adapters/Files/GridFSBucketAdapter.js

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -228,22 +228,35 @@ export class GridFSBucketAdapter extends FilesAdapter {
228228
const partialstart = parts[0];
229229
const partialend = parts[1];
230230

231-
const start = parseInt(partialstart, 10);
232-
const end = partialend ? parseInt(partialend, 10) : files[0].length - 1;
231+
const fileLength = files[0].length;
232+
const fileStart = parseInt(partialstart, 10);
233+
const fileEnd = partialend ? parseInt(partialend, 10) : fileLength;
233234

234-
res.writeHead(206, {
235-
'Accept-Ranges': 'bytes',
236-
'Content-Length': end - start + 1,
237-
'Content-Range': 'bytes ' + start + '-' + end + '/' + files[0].length,
238-
'Content-Type': contentType,
239-
});
235+
let start = Math.min(fileStart || 0, fileEnd, fileLength);
236+
let end = Math.max(fileStart || 0, fileEnd) + 1 || fileLength;
237+
if (isNaN(fileStart)) {
238+
start = fileLength - end + 1;
239+
end = fileLength;
240+
}
241+
end = Math.min(end, fileLength);
242+
start = Math.max(start, 0);
243+
244+
res.status(206);
245+
res.header('Accept-Ranges', 'bytes');
246+
res.header('Content-Length', end - start);
247+
res.header('Content-Range', 'bytes ' + start + '-' + end + '/' + fileLength);
248+
res.header('Content-Type', contentType);
240249
const stream = bucket.openDownloadStreamByName(filename);
241250
stream.start(start);
251+
if (end) {
252+
stream.end(end);
253+
}
242254
stream.on('data', chunk => {
243255
res.write(chunk);
244256
});
245-
stream.on('error', () => {
246-
res.sendStatus(404);
257+
stream.on('error', (e) => {
258+
res.status(404);
259+
res.send(e.message);
247260
});
248261
stream.on('end', () => {
249262
res.end();

‎src/Routers/FilesRouter.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,5 +243,10 @@ export class FilesRouter {
243243
}
244244

245245
function isFileStreamable(req, filesController) {
246-
return req.get('Range') && typeof filesController.adapter.handleFileStream === 'function';
246+
const range = (req.get('Range') || '/-/').split('-');
247+
const start = Number(range[0]);
248+
const end = Number(range[1]);
249+
return (
250+
(!isNaN(start) || !isNaN(end)) && typeof filesController.adapter.handleFileStream === 'function'
251+
);
247252
}

0 commit comments

Comments
 (0)