CVE-2022-3225: Improper Control of Dynamically-Managed Code Resources (CWE-913)
value: Improper Control of Dynamically-Managed Code Resources in GitHub repository budibase/budibase prior to 1.3.20.


CWE description: The product does not properly restrict reading from or writing to dynamically-managed code resources such as variables, objects, classes, attributes, functions, or executable instructions or statements.
CVSS v3 base score: 5.7
Published: 2022-09-16T17:15Z
Repository: https://github.com/budibase/budibase
Language: JavaScript
Files changed: packages/bbui/src/Tooltip/TooltipWrapper.svelte, packages/builder/src/builderStore/dataBinding.js, packages/builder/src/helpers/data/utils.js, packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte, packages/worker/src/api/controllers/global/self.js, packages/worker/src/api/controllers/global/users.ts
Commit date: 2022-09-15 19:35:55 +0100
Commit message: Fixing issue introduced by fix for #7683 - encoding the query string caused handlebars statements to break, this rectifies that.

Diff: diff --git a/packages/bbui/src/Tooltip/TooltipWrapper.svelte b/packages/bbui/src/Tooltip/TooltipWrapper.svelte
index 92f5c6f47..610b8382f 100644
--- a/packages/bbui/src/Tooltip/TooltipWrapper.svelte
+++ b/packages/bbui/src/Tooltip/TooltipWrapper.svelte
@@ -45,11 +45,11 @@
   .tooltip {
     position: absolute;
     display: flex;
     justify-content: center;
     top: 15px;
-    z-index: 100;
+    z-index: 200;
     width: 160px;
   }
   .icon {
     transform: scale(0.75);
   }
diff --git a/packages/builder/src/builderStore/dataBinding.js b/packages/builder/src/builderStore/dataBinding.js
index ed2c20950..36c88c162 100644
--- a/packages/builder/src/builderStore/dataBinding.js
+++ b/packages/builder/src/builderStore/dataBinding.js
@@ -7,18 +7,18 @@ import {
   getComponentSettings,
 } from "./componentUtils"
 import { store } from "builderStore"
 import {
   queries as queriesStores,
-  tables as tablesStore,
   roles as rolesStore,
+  tables as tablesStore,
 } from "stores/backend"
 import {
-  makePropSafe,
-  isJSBinding,
   decodeJSBinding,
   encodeJSBinding,
+  isJSBinding,
+  makePropSafe,
 } from "@budibase/string-templates"
 import { TableNames } from "../constants"
 import { JSONUtils } from "@budibase/frontend-core"
 import ActionDefinitions from "components/design/settings/controls/ButtonActionEditor/manifest.json"
 
@@ -116,12 +116,11 @@ export const toBindingsArray = (valueMap, prefix) => {
 export const readableToRuntimeMap = (bindings, ctx) => {
   if (!bindings || !ctx) {
     return {}
   }
   return Object.keys(ctx).reduce((acc, key) => {
-    let parsedQuery = readableToRuntimeBinding(bindings, ctx[key])
-    acc[key] = parsedQuery
+    acc[key] = readableToRuntimeBinding(bindings, ctx[key])
     return acc
   }, {})
 }
 
 /**
@@ -130,12 +129,11 @@ export const readableToRuntimeMap = (bindings, ctx) => {
 export const runtimeToReadableMap = (bindings, ctx) => {
   if (!bindings || !ctx) {
     return {}
   }
   return Object.keys(ctx).reduce((acc, key) => {
-    let parsedQuery = runtimeToReadableBinding(bindings, ctx[key])
-    acc[key] = parsedQuery
+    acc[key] = runtimeToReadableBinding(bindings, ctx[key])
     return acc
   }, {})
 }
 
 /**
diff --git a/packages/builder/src/helpers/data/utils.js b/packages/builder/src/helpers/data/utils.js
index cd6a8cf48..685ddf2a4 100644
--- a/packages/builder/src/helpers/data/utils.js
+++ b/packages/builder/src/helpers/data/utils.js
@@ -1,6 +1,7 @@
 import { IntegrationTypes } from "constants/backend"
+import { findHBSBlocks } from "@budibase/string-templates"
 
 export function schemaToFields(schema) {
   const response = {}
   if (schema && typeof schema === "object") {
     for (let [field, value] of Object.entries(schema)) {
@@ -29,11 +30,12 @@ export function breakQueryString(qs) {
   }
   const params = qs.split("&")
   let paramObj = {}
   for (let param of params) {
     const split = param.split("=")
-    paramObj[split[0]] = split.slice(1).join("=")
+    console.log(split[1])
+    paramObj[split[0]] = decodeURIComponent(split.slice(1).join("="))
   }
   return paramObj
 }
 
 export function buildQueryString(obj) {
@@ -44,11 +46,23 @@ export function buildQueryString(obj) {
         continue
       }
       if (str !== "") {
         str += "&"
       }
-      str += `${key}=${encodeURIComponent(value || "")}`
+      const bindings = findHBSBlocks(value)
+      let count = 0
+      const bindingMarkers = {}
+      bindings.forEach(binding => {
+        const marker = `BINDING...${count++}`
+        value = value.replace(binding, marker)
+        bindingMarkers[marker] = binding
+      })
+      let encoded = encodeURIComponent(value || "")
+      Object.entries(bindingMarkers).forEach(([marker, binding]) => {
+        encoded = encoded.replace(marker, binding)
+      })
+      str += `${key}=${encoded}`
     }
   }
   return str
 }
 
diff --git a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte
index d2c163041..6691494ac 100644
--- a/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte
+++ b/packages/builder/src/pages/builder/app/[application]/data/datasource/[selectedDatasource]/rest/[query]/index.svelte
@@ -345,10 +345,11 @@
 
     datasource = $datasources.list.find(ds => ds._id === query?.datasourceId)
     const datasourceUrl = datasource?.config.url
     const qs = query?.fields.queryString
     breakQs = restUtils.breakQueryString(qs)
+    console.log(breakQs)
     breakQs = runtimeToReadableMap(mergedBindings, breakQs)
 
     const path = query.fields.path
     if (
       datasourceUrl &&
@@ -706,10 +707,11 @@
     width: 960px;
   }
   .url-block {
     display: flex;
     gap: var(--spacing-s);
+    z-index: 200;
   }
   .verb {
     flex: 1;
   }
   .url {
diff --git a/packages/worker/src/api/controllers/global/self.js b/packages/worker/src/api/controllers/global/self.js
index 28afa69fa..9110e267f 100644
--- a/packages/worker/src/api/controllers/global/self.js
+++ b/packages/worker/src/api/controllers/global/self.js
@@ -78,20 +78,19 @@ const addSessionAttributesToUser = ctx => {
   ctx.body.budibaseAccess = !!ctx.user.budibaseAccess
   ctx.body.accountPortalAccess = !!ctx.user.accountPortalAccess
   ctx.body.csrfToken = ctx.user.csrfToken
 }
 
-/**
- * Remove the attributes that are session based from the current user,
- * so that stale values are not written to the db
- */
-const removeSessionAttributesFromUser = ctx => {
-  delete ctx.request.body.csrfToken
-  delete ctx.request.body.account
-  delete ctx.request.body.accountPortalAccess
-  delete ctx.request.body.budibaseAccess
-  delete ctx.request.body.license
+const sanitiseUserUpdate = ctx => {
+  const allowed = ["firstName", "lastName", "password", "forceResetPassword"]
+  const resp = {}
+  for (let [key, value] of Object.entries(ctx.request.body)) {
+    if (allowed.includes(key)) {
+      resp[key] = value
+    }
+  }
+  return resp
 }
 
 exports.getSelf = async ctx => {
   if (!ctx.user) {
     ctx.throw(403, "User not logged in")
@@ -115,29 +114,27 @@ exports.getSelf = async ctx => {
 
 exports.updateSelf = async ctx => {
   const db = getGlobalDB()
   const user = await db.get(ctx.user._id)
   let passwordChange = false
-  if (ctx.request.body.password) {
+
+  const userUpdateObj = sanitiseUserUpdate(ctx)
+  if (userUpdateObj.password) {
     // changing password
     passwordChange = true
-    ctx.request.body.password = await hash(ctx.request.body.password)
+    userUpdateObj.password = await hash(userUpdateObj.password)
     // Log all other sessions out apart from the current one
     await platformLogout({
       ctx,
       userId: ctx.user._id,
       keepActiveSession: true,
     })
   }
-  // don't allow sending up an ID/Rev, always use the existing one
-  delete ctx.request.body._id
-  delete ctx.request.body._rev
-  removeSessionAttributesFromUser(ctx)
 
   const response = await db.put({
     ...user,
-    ...ctx.request.body,
+    ...userUpdateObj,
   })
   await userCache.invalidateUser(user._id)
   ctx.body = {
     _id: response.id,
     _rev: response.rev,
diff --git a/packages/worker/src/api/controllers/global/users.ts b/packages/worker/src/api/controllers/global/users.ts
index d5e8eb8e6..ea9375f23 100644
--- a/packages/worker/src/api/controllers/global/users.ts
+++ b/packages/worker/src/api/controllers/global/users.ts
@@ -12,11 +12,10 @@ import {
   accounts,
   cache,
   errors,
   events,
   tenancy,
-  users as usersCore,
 } from "@budibase/backend-core"
 import { checkAnyUserExists } from "../../../utilities/users"
 import { groups as groupUtils } from "@budibase/pro"
 
 const MAX_USERS_UPLOAD_LIMIT = 1000
@@ -146,13 +145,11 @@ export const bulkDelete = async (ctx: any) => {
   if (userIds?.indexOf(ctx.user._id) !== -1) {
     ctx.throw(400, "Unable to delete self.")
   }
 
   try {
-    let response = await users.bulkDelete(userIds)
-
-    ctx.body = response
+    ctx.body = await users.bulkDelete(userIds)
   } catch (err) {
     ctx.throw(err)
   }
 }
 

Vulnerable code:     z-index: 100;
  tables as tablesStore,
  makePropSafe,
  isJSBinding,
    let parsedQuery = readableToRuntimeBinding(bindings, ctx[key])
    acc[key] = parsedQuery
    let parsedQuery = runtimeToReadableBinding(bindings, ctx[key])
    acc[key] = parsedQuery
    paramObj[split[0]] = split.slice(1).join("=")
      str += `${key}=${encodeURIComponent(value || "")}`
/**
 * Remove the attributes that are session based from the current user,
 * so that stale values are not written to the db
 */
const removeSessionAttributesFromUser = ctx => {
  delete ctx.request.body.csrfToken
  delete ctx.request.body.account
  delete ctx.request.body.accountPortalAccess
  delete ctx.request.body.budibaseAccess
  delete ctx.request.body.license
  if (ctx.request.body.password) {
    ctx.request.body.password = await hash(ctx.request.body.password)
  // don't allow sending up an ID/Rev, always use the existing one
  delete ctx.request.body._id
  delete ctx.request.body._rev
  removeSessionAttributesFromUser(ctx)
    ...ctx.request.body,
  users as usersCore,
    let response = await users.bulkDelete(userIds)

    ctx.body = response
Fixed code:     z-index: 200;
  tables as tablesStore,
  isJSBinding,
  makePropSafe,
    acc[key] = readableToRuntimeBinding(bindings, ctx[key])
    acc[key] = runtimeToReadableBinding(bindings, ctx[key])
import { findHBSBlocks } from "@budibase/string-templates"
    console.log(split[1])
    paramObj[split[0]] = decodeURIComponent(split.slice(1).join("="))
      const bindings = findHBSBlocks(value)
      let count = 0
      const bindingMarkers = {}
      bindings.forEach(binding => {
        const marker = `BINDING...${count++}`
        value = value.replace(binding, marker)
        bindingMarkers[marker] = binding
      })
      let encoded = encodeURIComponent(value || "")
      Object.entries(bindingMarkers).forEach(([marker, binding]) => {
        encoded = encoded.replace(marker, binding)
      })
      str += `${key}=${encoded}`
    console.log(breakQs)
    z-index: 200;
const sanitiseUserUpdate = ctx => {
  const allowed = ["firstName", "lastName", "password", "forceResetPassword"]
  const resp = {}
  for (let [key, value] of Object.entries(ctx.request.body)) {
    if (allowed.includes(key)) {
      resp[key] = value
    }
  }
  return resp

  const userUpdateObj = sanitiseUserUpdate(ctx)
  if (userUpdateObj.password) {
    userUpdateObj.password = await hash(userUpdateObj.password)
    ...userUpdateObj,
    ctx.body = await users.bulkDelete(userIds)