Skip to content

Commit 2f64f68

Browse files
ctcpipUlisesGascon
authored andcommitted
sec: security patch for CVE-2024-51999
1 parent ed0ba3f commit 2f64f68

File tree

2 files changed

+90
-3
lines changed

2 files changed

+90
-3
lines changed

lib/utils.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,6 @@ function createETagGenerator (options) {
266266

267267
function parseExtendedQueryString(str) {
268268
return qs.parse(str, {
269-
allowPrototypes: true
269+
plainObjects: true
270270
});
271271
}

test/req.query.js

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
var assert = require('node:assert')
44
var express = require('../')
55
, request = require('supertest');
6+
var qs = require('qs');
67

78
describe('req', function(){
89
describe('.query', function(){
@@ -38,6 +39,22 @@ describe('req', function(){
3839
.get('/?user.name=tj')
3940
.expect(200, '{"user.name":"tj"}', done);
4041
});
42+
43+
it('should not be able to access object prototype properties', function (done) {
44+
var app = createApp('extended', true);
45+
46+
request(app)
47+
.get('/?foo=yee')
48+
.expect(200, /TypeError: req\.query\.hasOwnProperty is not a function/, done);
49+
});
50+
51+
it('should be able to use object prototype property names as keys', function (done) {
52+
var app = createApp('extended', true);
53+
54+
request(app)
55+
.get('/?hasOwnProperty=yee')
56+
.expect(200, '{"query":{"hasOwnProperty":"yee"},"error":"TypeError: req.query.hasOwnProperty is not a function"}', done);
57+
});
4158
});
4259

4360
describe('when "query parser" is simple', function () {
@@ -48,6 +65,22 @@ describe('req', function(){
4865
.get('/?user%5Bname%5D=tj')
4966
.expect(200, '{"user[name]":"tj"}', done);
5067
});
68+
69+
it('should not be able to access object prototype properties', function (done) {
70+
var app = createApp('simple', true);
71+
72+
request(app)
73+
.get('/?foo=yee')
74+
.expect(200, /TypeError: req\.query\.hasOwnProperty is not a function/, done);
75+
});
76+
77+
it('should be able to use object prototype property names as keys', function (done) {
78+
var app = createApp('simple', true);
79+
80+
request(app)
81+
.get('/?hasOwnProperty=yee')
82+
.expect(200, '{"query":{"hasOwnProperty":"yee"},"error":"TypeError: req.query.hasOwnProperty is not a function"}', done);
83+
});
5184
});
5285

5386
describe('when "query parser" is a function', function () {
@@ -60,6 +93,18 @@ describe('req', function(){
6093
.get('/?user%5Bname%5D=tj')
6194
.expect(200, '{"length":17}', done);
6295
});
96+
97+
// test exists to verify behavior for folks wishing to workaround our qs defaults
98+
it('should drop object prototype property names and be able to access object prototype properties', function (done) {
99+
var app = createApp(
100+
function (str) {
101+
return qs.parse(str)
102+
}, true);
103+
104+
request(app)
105+
.get('/?hasOwnProperty=biscuits')
106+
.expect(200, '{"query":{},"hasOwnProperty":false}', done);
107+
});
63108
});
64109

65110
describe('when "query parser" disabled', function () {
@@ -70,6 +115,22 @@ describe('req', function(){
70115
.get('/?user%5Bname%5D=tj')
71116
.expect(200, '{}', done);
72117
});
118+
119+
it('should not be able to access object prototype properties', function (done) {
120+
var app = createApp('extended', true);
121+
122+
request(app)
123+
.get('/?foo=yee')
124+
.expect(200, /TypeError: req\.query\.hasOwnProperty is not a function/, done);
125+
});
126+
127+
it('should be able to use object prototype property names as keys', function (done) {
128+
var app = createApp('extended', true);
129+
130+
request(app)
131+
.get('/?hasOwnProperty=yee')
132+
.expect(200, '{"query":{"hasOwnProperty":"yee"},"error":"TypeError: req.query.hasOwnProperty is not a function"}', done);
133+
});
73134
});
74135

75136
describe('when "query parser" enabled', function () {
@@ -80,6 +141,22 @@ describe('req', function(){
80141
.get('/?user%5Bname%5D=tj')
81142
.expect(200, '{"user[name]":"tj"}', done);
82143
});
144+
145+
it('should not be able to access object prototype properties', function (done) {
146+
var app = createApp('extended', true);
147+
148+
request(app)
149+
.get('/?foo=yee')
150+
.expect(200, /TypeError: req\.query\.hasOwnProperty is not a function/, done);
151+
});
152+
153+
it('should be able to use object prototype property names as keys', function (done) {
154+
var app = createApp('extended', true);
155+
156+
request(app)
157+
.get('/?hasOwnProperty=yee')
158+
.expect(200, '{"query":{"hasOwnProperty":"yee"},"error":"TypeError: req.query.hasOwnProperty is not a function"}', done);
159+
});
83160
});
84161

85162
describe('when "query parser" an unknown value', function () {
@@ -91,15 +168,25 @@ describe('req', function(){
91168
})
92169
})
93170

94-
function createApp(setting) {
171+
function createApp(setting, isPrototypePropertyTest) {
95172
var app = express();
96173

97174
if (setting !== undefined) {
98175
app.set('query parser', setting);
99176
}
100177

101178
app.use(function (req, res) {
102-
res.send(req.query);
179+
if(isPrototypePropertyTest) {
180+
try {
181+
var hasOwnProperty = req.query.hasOwnProperty('✨ express ✨');
182+
res.send({ query: req.query, hasOwnProperty: hasOwnProperty });
183+
} catch (error) {
184+
res.send({ query: req.query, error: error.toString() });
185+
}
186+
}
187+
else {
188+
res.send(req.query);
189+
}
103190
});
104191

105192
return app;

0 commit comments

Comments
 (0)