You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
constexpress=require('express')constapp=express()constport=3000app.set('view engine','ejs');app.get('/',(req,res)=>{res.render('index',req.query);})app.listen(port,()=>{console.log(`Example app listening on port ${port}`)})
res.render=functionrender(view,options,callback){varapp=this.req.app;vardone=callback;varopts=options||{};varreq=this.req;varself=this;// support callback function as second argif(typeofoptions==='function'){done=options;opts={};}// merge res.localsopts._locals=self.locals;// default callback to responddone=done||function(err,str){if(err)returnreq.next(err);self.send(str);};// renderapp.render(view,opts,done);};
app.render=functionrender(name,options,callback){varcache=this.cache;vardone=callback;varengines=this.engines;varopts=options;varrenderOptions={};varview;// support callback function as second argif(typeofoptions==='function'){done=options;opts={};}// merge app.localsmerge(renderOptions,this.locals);// merge options._localsif(opts._locals){merge(renderOptions,opts._locals);}// merge optionsmerge(renderOptions,opts);// set .cache unless explicitly providedif(renderOptions.cache==null){renderOptions.cache=this.enabled('view cache');}// primed cacheif(renderOptions.cache){view=cache[name];}// viewif(!view){varView=this.get('view');view=newView(name,{defaultEngine: this.get('view engine'),root: this.get('views'),engines: engines});if(!view.path){vardirs=Array.isArray(view.root)&&view.root.length>1
? 'directories "'+view.root.slice(0,-1).join('", "')+'" or "'+view.root[view.root.length-1]+'"'
: 'directory "'+view.root+'"'varerr=newError('Failed to lookup view "'+name+'" in views '+dirs);err.view=view;returndone(err);}// prime the cacheif(renderOptions.cache){cache[name]=view;}}// rendertryRender(view,renderOptions,done);};
/** * Express.js support. * * This is an alias for {@link module:ejs.renderFile}, in order to support * Express.js out-of-the-box. * * @func */exports.__express=exports.renderFile;
exports.renderFile=function(){varargs=Array.prototype.slice.call(arguments);varfilename=args.shift();varcb;varopts={filename: filename};vardata;varviewOpts;// Do we have a callback?if(typeofarguments[arguments.length-1]=='function'){cb=args.pop();}// Do we have data/opts?if(args.length){// Should always have data objdata=args.shift();// Normal passed opts (data obj + opts obj)if(args.length){// Use shallowCopy so we don't pollute passed in opts obj with new valsutils.shallowCopy(opts,args.pop());}// Special casing for Express (settings + opts-in-data)else{// Express 3 and 4if(data.settings){// Pull a few things from known locationsif(data.settings.views){opts.views=data.settings.views;}if(data.settings['view cache']){opts.cache=true;}// Undocumented after Express 2, but still usable, esp. for// items that are unsafe to be passed along with data, like `root`viewOpts=data.settings['view options'];if(viewOpts){utils.shallowCopy(opts,viewOpts);}}// Express 2 and lower, values set in app.locals, or people who just// want to pass options in their data. NOTE: These values will override// anything previously set in settings or settings['view options']utils.shallowCopyFromList(opts,data,_OPTS_PASSABLE_WITH_DATA_EXPRESS);}opts.filename=filename;}else{data=utils.createNullProtoObjWherePossible();}returntryHandleCache(opts,data,cb);};
這邊的重點是中間那段:
if(data.settings){// Pull a few things from known locationsif(data.settings.views){opts.views=data.settings.views;}if(data.settings['view cache']){opts.cache=true;}// Undocumented after Express 2, but still usable, esp. for// items that are unsafe to be passed along with data, like `root`viewOpts=data.settings['view options'];if(viewOpts){utils.shallowCopy(opts,viewOpts);}}
簡單來說,設置 data.settings['view options'] 就可以蓋掉 opts。
再來一路往下追會到 handleCache:
functionhandleCache(options,template){varfunc;varfilename=options.filename;varhasTemplate=arguments.length>1;if(options.cache){if(!filename){thrownewError('cache option requires a filename');}func=exports.cache.get(filename);if(func){returnfunc;}if(!hasTemplate){template=fileLoader(filename).toString().replace(_BOM,'');}}elseif(!hasTemplate){// istanbul ignore if: should not happen at allif(!filename){thrownewError('Internal EJS error: no file name or template '+'provided');}template=fileLoader(filename).toString().replace(_BOM,'');}func=exports.compile(template,options);if(options.cache){exports.cache.set(filename,func);}returnfunc;}
The problem here is that EJS is simply a way of executing JS to render a template. If you allow passing of arbitrary/unsanitized options and data to the render function, you will encounter all security problems that would occur as a result of arbitrary code execution. Henny Youngman used to tell a joke: "The patient says, 'Doctor, it hurts when I do this.' So the doctor says, 'Then don't do that!'" I'm open to PRs that improve security, but this looks to me to be far beyond the purview of the library. These responsibilities live squarely in userland.
主要就是說如果開發者自己要這樣用 library 的話他也沒辦法,這不是 EJS 應該負責的,不該讓 end user 可以傳入整個 object。
Security professionals, before reporting any security issues, please reference the SECURITY.md in this project, in particular, the following: "EJS is effectively a JavaScript runtime. Its entire job is to execute JavaScript. If you run the EJS render method without checking the inputs yourself, you are responsible for the results."
原本是想用開發者的角度寫一篇,但最近沒什麼時間,先寫一篇用 CTF 角度來記錄這個問題,以後有時間再補上用開發者角度寫的。
簡單來說,這篇講的是使用以下 pattern 會造成的問題:
出現過的 CTF 考題
其實有關於 EJS,曾經考過的類型有兩種,第一種是像上面那樣你可以控制 render 中的第二個參數,第二種是你不能控制,但是有 prototype pollution 的漏洞。
第一種的話是 EJS 對於參數的處理其實我自己覺得有點問題,你可能以為只是傳入 data,但其實它 option 跟 data 是混在一起傳的,所以可以去更改 options,控制一些執行流程達成 RCE。
第二種的話主要是經由 prototype pollution 去污染
outputFunctionName
,然後靠著 EJS 底層會用outputFunctionName
去拼接 JS 程式碼達成 RCE。不過後來 EJS 有新增對於
outputFunctionName
的檢查就是了,確保傳入的東西真的是個合法的變數名稱。這篇文章談的主要是第一種的狀況。
底下是以前出現過的相關考題,早期比較多都是 prototype pollution 為主,最近出現的則是直接讓你傳入 object 居多。
問題的根源
呼叫
res.render()
以後會先到 express/lib/response.js:接著看到
app.render
,在 express/lib/application.js:這邊最後會呼叫
tryRender
,程式碼在express/lib/application.js:這個
view.render
會去呼叫 view engine 裡面的__express
方法,而這個方法在 EJS 裡面就是renderFile
:ejs/lib/ejs.js:
renderFile:
這邊的重點是中間那段:
簡單來說,設置
data.settings['view options']
就可以蓋掉opts
。再來一路往下追會到
handleCache
:如果
options.cache
有設置,那就直接用 cache 裡已經 compile 過的東西,否則就重新 compile 一次。而最後重點中的重點就是 compile,裡面有一段如下:
會拿
escapeFn
去拼接程式碼。於是我們只要傳入:
就可以執行任意程式碼,達成 RCE。
Cache 的問題
雖然前面講的很順,但是有一個 cache 的問題。
在 production 模式底下 view cache 會自動啟用:
而這個參數在 render 的時候會自動被帶到 options 裡面:
雖然說我們可以透過 view options 覆蓋原本的 options,但如果原本 options 內就有傳入 cache 的話,又會被覆蓋回去:
如果無法覆蓋 cache,那就不能使用上面的方法了,因為 template 不會重新被 compile。
不過沒關係,幸好這是 JavaScript,注意這行程式碼:
如果
renderOptions.cache
是 null 的話才會去設置,而0 == null
是 false,所以我們可以傳入cache: 0
,就不會進去這一段。而
0
是 falsy,所以可以繞過 cache 的檢查,讓if (options.cache)
是 false。EJS 作者的看法
其實 EJS 從以前就有不少相關的 issue 了,清單如下:
而作者的立場從以前到現在都一樣:
主要就是說如果開發者自己要這樣用 library 的話他也沒辦法,這不是 EJS 應該負責的,不該讓 end user 可以傳入整個 object。
而 EJS 的開發者最近也因為收到很多這類型的 issue report,直接在 README 以及官網上面都加上了告示:
所以這篇文章講的這招無論是現在還是未來都可以用,只要看到有人在 render 時的 object 可控,就意味著可以打到 RCE。
之後想再寫一篇從開發者角度來看這件事情,雖然說 EJS 作者講的有點道理,但至少 EJS 作為一個 library,應該在文件上特別提醒開發者不該這樣使用,雖然說現在已經有提示,但更針對的是叫 security researcher 不要回報,而不是叫開發者不要這樣用。
或者,會不會這個其實是一個 bad coding practice,一開始就不該有這樣的 pattern 可以讓別人利用?
這塊我也還沒想清楚,之後想清楚再來寫吧。
The text was updated successfully, but these errors were encountered: