Incorrect authentication strategy query

[mbaluda]

Relevant sources:

• CAP: Authentication Strategies: Dummy
• CWE CATEGORY: Comprehensive Categorization: Access Control
• CWE-798: Use of Hard-coded Credentials
• CWE-259: Use of Hard-coded Password
• CWE-1392: Use of Default Credentials

Source code of cds.User:

class User {

  constructor (_) {
    if (_ === undefined) {
      if (new.target === Privileged) return
      if (new.target === Anonymous) return
      else return new User.default
    }
    if (typeof _ === 'string') { this.id = _; return }
    for (let each in _) super[each === '_roles' ? 'roles' : each] = _[each] // overrides getters
    const roles = this.hasOwnProperty('roles') && this.roles // eslint-disable-line no-prototype-builtins
    if (Array.isArray(roles)) this.roles = roles.filter(r => !PSEUDO_ROLES.includes(r)).reduce ((p,n)=>{p[n]=1; return p},{})
    else PSEUDO_ROLES.forEach(r => delete this.roles[r])
  }

  get attr() { return super.attr = {} }
  get roles(){ return super.roles = {} }
  get _roles(){ return this.roles } // compatibility

  is (role) {
    return role === 'any' ||
      role === 'identified-user' ||
      role === 'system-user' && this._is_system ||
      role === 'internal-user' && this._is_internal ||
      role === 'authenticated-user' ||
      !!this.roles[role]
  }
  valueOf() { return this.id }

}

The above source shows that 'any', 'identified-user', 'system-user', 'internal-user', and 'authenticated-user' are built-in, hardcoded.
Relevant documentation: CAP: Authorization: Pseudo Roles

[mbaluda]

Auth dummy should only be used at development time

[jeongsoolee09]

There is a class of cds.User called cds.User.Privileged: It looks like it should never be misused.

[jeongsoolee09]

The developer can add a custom authentication strategy that should satisfy certain criteria: link. We can see if that is actually true.

[jeongsoolee09]

The cds.User.is is insecure if left as a constant true function as in this example:

const cds = require('@sap/cds')
const DummyUser = new class extends cds.User { is:()=>true }
module.exports = (req,res,next) => {
  req.user = new DummyUser('dummy')
  next()
}

We can indeed check the above by looking at the implementation of cds.User.Privileged, which passes all authentication measures:

class Privileged extends User { is(){ return true }}

[jeongsoolee09]

Query to find constant true functions, trying best to capture them statically:

from Function function
where forall(Expr expr | expr = function.getAReturnedExpr() | expr.mayHaveBooleanValue(true))
select function

[jeongsoolee09]

In theory the constructor call like below can happen anywhere:

const cds = require('@sap/cds')
// with user ID as string
const user = new cds.User('userId')
// a user instance
const anotherUser = new cds.User(user)
// a user instance like object
const yetAnotherUser = new cds.User({id: user.id, roles: user.roles, attr: user.attr})

But in reality it can happen either in a custom authentication middleware like this:

const cds = require('@sap/cds')
const DummyUser = new class extends cds.User { is:()=>true }
module.exports = (req,res,next) => {
    req.user = new DummyUser('dummy')
    next()
}

Or in a handler registration like this:

this.before("*", function (req) {
  const user = new cds.User.Privileged();
  return this.tx({ user }, (tx) =>
    tx.run(
      INSERT.into("RequestLog").entries({
        url: req._.req.url,
        user: req.user.id,
      }),
    ),
  );
});