Skip to content

Commit 4c1befa

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)) [skip release] (#8237)
1 parent 1a2b1b9 commit 4c1befa

File tree

3 files changed

+228
-21
lines changed

3 files changed

+228
-21
lines changed

Diff for: ‎spec/ParseFile.spec.js

+199-10
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,198 @@ describe('Parse.File testing', () => {
692692
});
693693
});
694694

695-
xdescribe('Gridstore Range tests', () => {
695+
describe_only_db('mongo')('Gridstore Range', () => {
696+
it('supports bytes range out of range', async () => {
697+
const headers = {
698+
'Content-Type': 'application/octet-stream',
699+
'X-Parse-Application-Id': 'test',
700+
'X-Parse-REST-API-Key': 'rest',
701+
};
702+
const response = await request({
703+
method: 'POST',
704+
headers: headers,
705+
url: 'http://localhost:8378/1//files/file.txt ',
706+
body: repeat('argle bargle', 100),
707+
});
708+
const b = response.data;
709+
const file = await request({
710+
url: b.url,
711+
headers: {
712+
'Content-Type': 'application/octet-stream',
713+
'X-Parse-Application-Id': 'test',
714+
Range: 'bytes=15000-18000',
715+
},
716+
}).catch(e => e);
717+
expect(file.headers['content-range']).toBe('bytes 1212-1212/1212');
718+
});
719+
720+
it('supports bytes range if end greater than start', async () => {
721+
const headers = {
722+
'Content-Type': 'application/octet-stream',
723+
'X-Parse-Application-Id': 'test',
724+
'X-Parse-REST-API-Key': 'rest',
725+
};
726+
const response = await request({
727+
method: 'POST',
728+
headers: headers,
729+
url: 'http://localhost:8378/1//files/file.txt ',
730+
body: repeat('argle bargle', 100),
731+
});
732+
const b = response.data;
733+
const file = await request({
734+
url: b.url,
735+
headers: {
736+
'Content-Type': 'application/octet-stream',
737+
'X-Parse-Application-Id': 'test',
738+
Range: 'bytes=15000-100',
739+
},
740+
});
741+
expect(file.headers['content-range']).toBe('bytes 100-1212/1212');
742+
});
743+
744+
it('supports bytes range if end is undefined', async () => {
745+
const headers = {
746+
'Content-Type': 'application/octet-stream',
747+
'X-Parse-Application-Id': 'test',
748+
'X-Parse-REST-API-Key': 'rest',
749+
};
750+
const response = await request({
751+
method: 'POST',
752+
headers: headers,
753+
url: 'http://localhost:8378/1//files/file.txt ',
754+
body: repeat('argle bargle', 100),
755+
});
756+
const b = response.data;
757+
const file = await request({
758+
url: b.url,
759+
headers: {
760+
'Content-Type': 'application/octet-stream',
761+
'X-Parse-Application-Id': 'test',
762+
Range: 'bytes=100-',
763+
},
764+
});
765+
expect(file.headers['content-range']).toBe('bytes 100-1212/1212');
766+
});
767+
768+
it('supports bytes range if start and end undefined', async () => {
769+
const headers = {
770+
'Content-Type': 'application/octet-stream',
771+
'X-Parse-Application-Id': 'test',
772+
'X-Parse-REST-API-Key': 'rest',
773+
};
774+
const response = await request({
775+
method: 'POST',
776+
headers: headers,
777+
url: 'http://localhost:8378/1//files/file.txt ',
778+
body: repeat('argle bargle', 100),
779+
});
780+
const b = response.data;
781+
const file = await request({
782+
url: b.url,
783+
headers: {
784+
'Content-Type': 'application/octet-stream',
785+
'X-Parse-Application-Id': 'test',
786+
Range: 'bytes=abc-efs',
787+
},
788+
}).catch(e => e);
789+
expect(file.headers['content-range']).toBeUndefined();
790+
});
791+
792+
it('supports bytes range if start and end undefined', async () => {
793+
const headers = {
794+
'Content-Type': 'application/octet-stream',
795+
'X-Parse-Application-Id': 'test',
796+
'X-Parse-REST-API-Key': 'rest',
797+
};
798+
const response = await request({
799+
method: 'POST',
800+
headers: headers,
801+
url: 'http://localhost:8378/1//files/file.txt ',
802+
body: repeat('argle bargle', 100),
803+
});
804+
const b = response.data;
805+
const file = await request({
806+
url: b.url,
807+
headers: {
808+
'Content-Type': 'application/octet-stream',
809+
'X-Parse-Application-Id': 'test',
810+
},
811+
}).catch(e => e);
812+
expect(file.headers['content-range']).toBeUndefined();
813+
});
814+
815+
it('supports bytes range if end is greater than size', async () => {
816+
const headers = {
817+
'Content-Type': 'application/octet-stream',
818+
'X-Parse-Application-Id': 'test',
819+
'X-Parse-REST-API-Key': 'rest',
820+
};
821+
const response = await request({
822+
method: 'POST',
823+
headers: headers,
824+
url: 'http://localhost:8378/1//files/file.txt ',
825+
body: repeat('argle bargle', 100),
826+
});
827+
const b = response.data;
828+
const file = await request({
829+
url: b.url,
830+
headers: {
831+
'Content-Type': 'application/octet-stream',
832+
'X-Parse-Application-Id': 'test',
833+
Range: 'bytes=0-2000',
834+
},
835+
}).catch(e => e);
836+
expect(file.headers['content-range']).toBe('bytes 0-1212/1212');
837+
});
838+
839+
it('supports bytes range if end is greater than size', async () => {
840+
const headers = {
841+
'Content-Type': 'application/octet-stream',
842+
'X-Parse-Application-Id': 'test',
843+
'X-Parse-REST-API-Key': 'rest',
844+
};
845+
const response = await request({
846+
method: 'POST',
847+
headers: headers,
848+
url: 'http://localhost:8378/1//files/file.txt ',
849+
body: repeat('argle bargle', 100),
850+
});
851+
const b = response.data;
852+
const file = await request({
853+
url: b.url,
854+
headers: {
855+
'Content-Type': 'application/octet-stream',
856+
'X-Parse-Application-Id': 'test',
857+
Range: 'bytes=0-2000',
858+
},
859+
}).catch(e => e);
860+
expect(file.headers['content-range']).toBe('bytes 0-1212/1212');
861+
});
862+
863+
it('supports bytes range with 0 length', async () => {
864+
const headers = {
865+
'Content-Type': 'application/octet-stream',
866+
'X-Parse-Application-Id': 'test',
867+
'X-Parse-REST-API-Key': 'rest',
868+
};
869+
const response = await request({
870+
method: 'POST',
871+
headers: headers,
872+
url: 'http://localhost:8378/1//files/file.txt ',
873+
body: 'a',
874+
}).catch(e => e);
875+
const b = response.data;
876+
const file = await request({
877+
url: b.url,
878+
headers: {
879+
'Content-Type': 'application/octet-stream',
880+
'X-Parse-Application-Id': 'test',
881+
Range: 'bytes=-2000',
882+
},
883+
}).catch(e => e);
884+
expect(file.headers['content-range']).toBe('bytes 0-1/1');
885+
});
886+
696887
it('supports range requests', done => {
697888
const headers = {
698889
'Content-Type': 'application/octet-stream',
@@ -781,7 +972,7 @@ describe('Parse.File testing', () => {
781972
});
782973
});
783974

784-
xit('supports getting last n bytes', done => {
975+
it('supports getting last n bytes', done => {
785976
const headers = {
786977
'Content-Type': 'application/octet-stream',
787978
'X-Parse-Application-Id': 'test',
@@ -879,21 +1070,19 @@ describe('Parse.File testing', () => {
8791070
});
8801071
});
8811072

882-
it('fails to stream unknown file', done => {
883-
request({
1073+
it('fails to stream unknown file', async () => {
1074+
const response = await request({
8841075
url: 'http://localhost:8378/1/files/test/file.txt',
8851076
headers: {
8861077
'Content-Type': 'application/octet-stream',
8871078
'X-Parse-Application-Id': 'test',
8881079
'X-Parse-REST-API-Key': 'rest',
8891080
Range: 'bytes=13-240',
8901081
},
891-
}).then(response => {
892-
expect(response.status).toBe(404);
893-
const body = response.text;
894-
expect(body).toEqual('File not found.');
895-
done();
896-
});
1082+
}).catch(e => e);
1083+
expect(response.status).toBe(404);
1084+
const body = response.text;
1085+
expect(body).toEqual('File not found.');
8971086
});
8981087
});
8991088

Diff for: ‎src/Adapters/Files/GridFSBucketAdapter.js

+23-10
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();

Diff for: ‎src/Routers/FilesRouter.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -266,5 +266,10 @@ export class FilesRouter {
266266
}
267267

268268
function isFileStreamable(req, filesController) {
269-
return req.get('Range') && typeof filesController.adapter.handleFileStream === 'function';
269+
const range = (req.get('Range') || '/-/').split('-');
270+
const start = Number(range[0]);
271+
const end = Number(range[1]);
272+
return (
273+
(!isNaN(start) || !isNaN(end)) && typeof filesController.adapter.handleFileStream === 'function'
274+
);
270275
}

0 commit comments

Comments
 (0)