diff --git a/lib/messages.js b/lib/messages.js index a785dcc9..367d08f7 100755 --- a/lib/messages.js +++ b/lib/messages.js @@ -36,8 +36,10 @@ exports.compile = function (messages, target) { if (code === 'root' || Template.isTemplate(message)) { - // A flat code named __proto__ triggers the legacy accessor on plain-object - // assignment, replacing target's own prototype instead of creating an own property + // A flat code named __proto__ triggers the legacy accessor on plain-object assignment, and + // what we assign is always a Template, i.e. an object the accessor accepts, so it replaces + // target's own prototype instead of creating an own property. That in turn makes + // Template.isTemplate(target) true, so every code renders that one message assert(code !== '__proto__', 'Cannot use __proto__ as a message code'); diff --git a/test/base.js b/test/base.js index 3e9e0732..61926a08 100755 --- a/test/base.js +++ b/test/base.js @@ -2108,6 +2108,30 @@ describe('any', () => { } }); + it('rejects a flat message code named __proto__', () => { + + // The message is wrapped in a Template before the assignment, and a Template is an object, + // so the inherited setter takes it and replaces the returned object's own prototype. That + // makes Template.isTemplate() true for the whole messages object, so every code renders the + // attacker's message and the next compile() throws 'Cannot set single message template' + + for (const message of ['pwned', Joi.x('pwned')]) { + const messages = { ['__proto__']: message }; + + expect(() => Joi.number().min(10).messages(messages)).to.throw('Cannot use __proto__ as a message code'); + expect(() => Joi.number().min(10).prefs({ messages })).to.throw('Cannot use __proto__ as a message code'); + expect(() => Joi.number().min(10).validate(1, { messages })).to.throw('Cannot use __proto__ as a message code'); + } + }); + + it('rejects a language scoped message code named __proto__', () => { + + const messages = { english: { ['__proto__']: 'pwned' } }; + + expect(() => Joi.number().min(10).messages(messages)).to.throw('Cannot use __proto__ as a message code'); + expect(() => Joi.number().min(10).messages({ english: { ['__proto__']: Joi.x('pwned') } })).to.throw('Cannot use __proto__ as a message code'); + }); + it('errors on invalid message value', () => { expect(() => Joi.number().min(10).message(12)).to.throw('Invalid message options'); diff --git a/test/extend.js b/test/extend.js index c5645c62..68b7c6d6 100755 --- a/test/extend.js +++ b/test/extend.js @@ -584,6 +584,24 @@ describe('extension', () => { } }); + it('rejects a flat message code named __proto__', () => { + + // Same as compile(), the Template lands on the merged object's own prototype + + for (const message of ['pwned', Joi.x('pwned')]) { + const extend = () => Joi.extend({ type: 'special', base: Joi.string(), messages: { ['__proto__']: message } }); + + expect(extend).to.throw('Cannot use __proto__ as a message code'); + } + }); + + it('rejects a language scoped message code named __proto__', () => { + + const extend = () => Joi.extend({ type: 'special', base: Joi.string(), messages: { english: { ['__proto__']: 'pwned' } } }); + + expect(extend).to.throw('Cannot use __proto__ as a message code'); + }); + it('overrides specific error messages with template', () => { const custom = Joi.extend({ diff --git a/test/messages_proto_poc.js b/test/messages_proto_poc.js deleted file mode 100644 index 80ad3072..00000000 --- a/test/messages_proto_poc.js +++ /dev/null @@ -1,44 +0,0 @@ -'use strict'; - -const Code = require('@hapi/code'); -const Lab = require('@hapi/lab'); - -const Joi = require('..'); -const Messages = require('../lib/messages'); - -const { describe, it } = exports.lab = Lab.script(); -const expect = Code.expect; - -// JSON.parse produces a genuine own "__proto__" property (unlike the { __proto__: ... } -// object literal shorthand, which the parser special-cases as prototype-setting syntax -// and never creates an own key), matching how an attacker delivers this in practice -// (a JSON request body or config file fed into .messages()/.prefs()). -const attackerJson = (message) => JSON.parse(`{"__proto__": ${JSON.stringify(message)}}`); - - -describe('messages() proto guard', () => { - - it('does not let a top-level __proto__ code hijack the compiled messages object prototype', () => { - - const before = Messages.compile({ 'number.min': 'too small' }); - expect(Object.getPrototypeOf(before)).to.equal(Object.prototype); - - expect(() => Messages.compile(attackerJson('pwned'))).to.throw(); - }); - - it('rejects __proto__ as a language-scoped error code', () => { - - expect(() => Messages.compile({ english: attackerJson('pwned') })).to.throw(); - }); - - it('rejects __proto__ via merge()', () => { - - expect(() => Messages.merge({ 'number.min': 'too small' }, attackerJson('pwned'))).to.throw(); - }); - - it('rejects __proto__ through the public .messages()/.prefs() schema API', () => { - - expect(() => Joi.any().messages(attackerJson('pwned'))).to.throw(); - expect(() => Joi.any().prefs({ messages: attackerJson('pwned') })).to.throw(); - }); -});