CVE-2023-50710: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: Hono is a web framework written in TypeScript. Prior to version 3.11.7, clients may override named path parameter values from previous requests if the application is using TrieRouter. So, there is a risk that a privileged user may use unintended parameters when deleting REST API resources. TrieRouter is used either explicitly or when the application matches a pattern that is not supported by the default RegExpRouter. Version 3.11.7 includes the change to fix this issue. As a workaround, avoid using TrieRouter directly.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 4.3
Published: 2023-12-14T18:15Z
Repository: https://github.com/honojs/hono
Language: TypeScript
Files changed: deno_dist/router/trie-router/node.ts, src/router/trie-router/node.test.ts, src/router/trie-router/node.ts
Commit date: 2023-12-14 10:21:02 +0900
Commit message: Merge pull request from GHSA-f6gv-hh8j-q8vq

* fix(trie-router): don't remain with the values of named param

* denoify

* fix: don't share `params`

* denoify
Diff: diff --git a/deno_dist/router/trie-router/node.ts b/deno_dist/router/trie-router/node.ts
index f9de7d5..c55a23a 100644
--- a/deno_dist/router/trie-router/node.ts
+++ b/deno_dist/router/trie-router/node.ts
@@ -3,16 +3,19 @@ import { METHOD_NAME_ALL } from '../../router.ts'
 import type { Pattern } from '../../utils/url.ts'
 import { splitPath, splitRoutingPath, getPattern } from '../../utils/url.ts'
 
 type HandlerSet<T> = {
   handler: T
-  params: Record<string, string>
   possibleKeys: string[]
   score: number
   name: string // For debug
 }
 
+type HandlerParamsSet<T> = HandlerSet<T> & {
+  params: Record<string, string>
+}
+
 export class Node<T> {
   methods: Record<string, HandlerSet<T>>[]
 
   children: Record<string, Node<T>>
   patterns: Pattern[]
@@ -24,11 +27,11 @@ export class Node<T> {
     this.children = children || {}
     this.methods = []
     this.name = ''
     if (method && handler) {
       const m: Record<string, HandlerSet<T>> = {}
-      m[method] = { handler, params: {}, possibleKeys: [], score: 0, name: this.name }
+      m[method] = { handler, possibleKeys: [], score: 0, name: this.name }
       this.methods = [m]
     }
     this.patterns = []
   }
 
@@ -72,11 +75,10 @@ export class Node<T> {
 
     const m: Record<string, HandlerSet<T>> = {}
 
     const handlerSet: HandlerSet<T> = {
       handler,
-      params: {},
       possibleKeys,
       name: this.name,
       score: this.order,
     }
 
@@ -85,27 +87,32 @@ export class Node<T> {
 
     return curNode
   }
 
   // getHandlerSets
-  private gHSets(node: Node<T>, method: string, params: Record<string, string>): HandlerSet<T>[] {
-    const handlerSets: HandlerSet<T>[] = []
+  private gHSets(
+    node: Node<T>,
+    method: string,
+    params: Record<string, string>
+  ): HandlerParamsSet<T>[] {
+    const handlerSets: HandlerParamsSet<T>[] = []
     for (let i = 0, len = node.methods.length; i < len; i++) {
       const m = node.methods[i]
-      const handlerSet = m[method] || m[METHOD_NAME_ALL]
+      const handlerSet = (m[method] || m[METHOD_NAME_ALL]) as HandlerParamsSet<T>
       if (handlerSet !== undefined) {
+        handlerSet.params = {}
         handlerSet.possibleKeys.map((key) => {
           handlerSet.params[key] = params[key]
         })
         handlerSets.push(handlerSet)
       }
     }
     return handlerSets
   }
 
   search(method: string, path: string): [[T, Params][]] {
-    const handlerSets: HandlerSet<T>[] = []
+    const handlerSets: HandlerParamsSet<T>[] = []
 
     const params: Record<string, string> = {}
     this.params = {}
 
     // eslint-disable-next-line @typescript-eslint/no-this-alias
@@ -124,15 +131,13 @@ export class Node<T> {
 
         if (nextNode) {
           if (isLast === true) {
             // '/hello/*' => match '/hello'
             if (nextNode.children['*']) {
-              handlerSets.push(
-                ...this.gHSets(nextNode.children['*'], method, { ...params, ...node.params })
-              )
+              handlerSets.push(...this.gHSets(nextNode.children['*'], method, node.params))
             }
-            handlerSets.push(...this.gHSets(nextNode, method, { ...params, ...node.params }))
+            handlerSets.push(...this.gHSets(nextNode, method, node.params))
           } else {
             tempNodes.push(nextNode)
           }
         }
 
@@ -142,11 +147,11 @@ export class Node<T> {
           // Wildcard
           // '/hello/*/foo' => match /hello/bar/foo
           if (pattern === '*') {
             const astNode = node.children['*']
             if (astNode) {
-              handlerSets.push(...this.gHSets(astNode, method, { ...params, ...node.params }))
+              handlerSets.push(...this.gHSets(astNode, method, node.params))
               tempNodes.push(astNode)
             }
             continue
           }
 
diff --git a/src/router/trie-router/node.test.ts b/src/router/trie-router/node.test.ts
index 1978eef..7941093 100644
--- a/src/router/trie-router/node.test.ts
+++ b/src/router/trie-router/node.test.ts
@@ -87,10 +87,25 @@ describe('Name path', () => {
     const [res] = node.search('get', '/map/yokohama/events')
     expect(res.length).toBe(1)
     expect(res[0][0]).toEqual('get events')
     expect(res[0][1]['location']).toBe('yokohama')
   })
+
+  it('Should not return a previous param value', () => {
+    const node = new Node()
+    node.insert('delete', '/resource/:id', 'resource')
+    const [resA] = node.search('delete', '/resource/a')
+    const [resB] = node.search('delete', '/resource/b')
+    expect(resA).not.toBeNull()
+    expect(resA.length).toBe(1)
+    expect(resA[0][0]).toEqual('resource')
+    expect(resA[0][1]).toEqual({ id: 'a' })
+    expect(resB).not.toBeNull()
+    expect(resB.length).toBe(1)
+    expect(resB[0][0]).toEqual('resource')
+    expect(resB[0][1]).toEqual({ id: 'b' })
+  })
 })
 
 describe('Name path - Multiple route', () => {
   const node = new Node()
 
diff --git a/src/router/trie-router/node.ts b/src/router/trie-router/node.ts
index 2a34266..7b120d4 100644
--- a/src/router/trie-router/node.ts
+++ b/src/router/trie-router/node.ts
@@ -3,16 +3,19 @@ import { METHOD_NAME_ALL } from '../../router'
 import type { Pattern } from '../../utils/url'
 import { splitPath, splitRoutingPath, getPattern } from '../../utils/url'
 
 type HandlerSet<T> = {
   handler: T
-  params: Record<string, string>
   possibleKeys: string[]
   score: number
   name: string // For debug
 }
 
+type HandlerParamsSet<T> = HandlerSet<T> & {
+  params: Record<string, string>
+}
+
 export class Node<T> {
   methods: Record<string, HandlerSet<T>>[]
 
   children: Record<string, Node<T>>
   patterns: Pattern[]
@@ -24,11 +27,11 @@ export class Node<T> {
     this.children = children || {}
     this.methods = []
     this.name = ''
     if (method && handler) {
       const m: Record<string, HandlerSet<T>> = {}
-      m[method] = { handler, params: {}, possibleKeys: [], score: 0, name: this.name }
+      m[method] = { handler, possibleKeys: [], score: 0, name: this.name }
       this.methods = [m]
     }
     this.patterns = []
   }
 
@@ -72,11 +75,10 @@ export class Node<T> {
 
     const m: Record<string, HandlerSet<T>> = {}
 
     const handlerSet: HandlerSet<T> = {
       handler,
-      params: {},
       possibleKeys,
       name: this.name,
       score: this.order,
     }
 
@@ -85,27 +87,32 @@ export class Node<T> {
 
     return curNode
   }
 
   // getHandlerSets
-  private gHSets(node: Node<T>, method: string, params: Record<string, string>): HandlerSet<T>[] {
-    const handlerSets: HandlerSet<T>[] = []
+  private gHSets(
+    node: Node<T>,
+    method: string,
+    params: Record<string, string>
+  ): HandlerParamsSet<T>[] {
+    const handlerSets: HandlerParamsSet<T>[] = []
     for (let i = 0, len = node.methods.length; i < len; i++) {
       const m = node.methods[i]
-      const handlerSet = m[method] || m[METHOD_NAME_ALL]
+      const handlerSet = (m[method] || m[METHOD_NAME_ALL]) as HandlerParamsSet<T>
       if (handlerSet !== undefined) {
+        handlerSet.params = {}
         handlerSet.possibleKeys.map((key) => {
           handlerSet.params[key] = params[key]
         })
         handlerSets.push(handlerSet)
       }
     }
     return handlerSets
   }
 
   search(method: string, path: string): [[T, Params][]] {
-    const handlerSets: HandlerSet<T>[] = []
+    const handlerSets: HandlerParamsSet<T>[] = []
 
     const params: Record<string, string> = {}
     this.params = {}
 
     // eslint-disable-next-line @typescript-eslint/no-this-alias
@@ -124,15 +131,13 @@ export class Node<T> {
 
         if (nextNode) {
           if (isLast === true) {
             // '/hello/*' => match '/hello'
             if (nextNode.children['*']) {
-              handlerSets.push(
-                ...this.gHSets(nextNode.children['*'], method, { ...params, ...node.params })
-              )
+              handlerSets.push(...this.gHSets(nextNode.children['*'], method, node.params))
             }
-            handlerSets.push(...this.gHSets(nextNode, method, { ...params, ...node.params }))
+            handlerSets.push(...this.gHSets(nextNode, method, node.params))
           } else {
             tempNodes.push(nextNode)
           }
         }
 
@@ -142,11 +147,11 @@ export class Node<T> {
           // Wildcard
           // '/hello/*/foo' => match /hello/bar/foo
           if (pattern === '*') {
             const astNode = node.children['*']
             if (astNode) {
-              handlerSets.push(...this.gHSets(astNode, method, { ...params, ...node.params }))
+              handlerSets.push(...this.gHSets(astNode, method, node.params))
               tempNodes.push(astNode)
             }
             continue
           }
 

Vulnerable code:   params: Record<string, string>
      m[method] = { handler, params: {}, possibleKeys: [], score: 0, name: this.name }
      params: {},
  private gHSets(node: Node<T>, method: string, params: Record<string, string>): HandlerSet<T>[] {
    const handlerSets: HandlerSet<T>[] = []
      const handlerSet = m[method] || m[METHOD_NAME_ALL]
    const handlerSets: HandlerSet<T>[] = []
              handlerSets.push(
                ...this.gHSets(nextNode.children['*'], method, { ...params, ...node.params })
              )
            handlerSets.push(...this.gHSets(nextNode, method, { ...params, ...node.params }))
              handlerSets.push(...this.gHSets(astNode, method, { ...params, ...node.params }))
  params: Record<string, string>
      m[method] = { handler, params: {}, possibleKeys: [], score: 0, name: this.name }
      params: {},
  private gHSets(node: Node<T>, method: string, params: Record<string, string>): HandlerSet<T>[] {
    const handlerSets: HandlerSet<T>[] = []
      const handlerSet = m[method] || m[METHOD_NAME_ALL]
    const handlerSets: HandlerSet<T>[] = []
              handlerSets.push(
                ...this.gHSets(nextNode.children['*'], method, { ...params, ...node.params })
              )
            handlerSets.push(...this.gHSets(nextNode, method, { ...params, ...node.params }))
              handlerSets.push(...this.gHSets(astNode, method, { ...params, ...node.params }))
Fixed code: type HandlerParamsSet<T> = HandlerSet<T> & {
  params: Record<string, string>
}

      m[method] = { handler, possibleKeys: [], score: 0, name: this.name }
  private gHSets(
    node: Node<T>,
    method: string,
    params: Record<string, string>
  ): HandlerParamsSet<T>[] {
    const handlerSets: HandlerParamsSet<T>[] = []
      const handlerSet = (m[method] || m[METHOD_NAME_ALL]) as HandlerParamsSet<T>
        handlerSet.params = {}
    const handlerSets: HandlerParamsSet<T>[] = []
              handlerSets.push(...this.gHSets(nextNode.children['*'], method, node.params))
            handlerSets.push(...this.gHSets(nextNode, method, node.params))
              handlerSets.push(...this.gHSets(astNode, method, node.params))

  it('Should not return a previous param value', () => {
    const node = new Node()
    node.insert('delete', '/resource/:id', 'resource')
    const [resA] = node.search('delete', '/resource/a')
    const [resB] = node.search('delete', '/resource/b')
    expect(resA).not.toBeNull()
    expect(resA.length).toBe(1)
    expect(resA[0][0]).toEqual('resource')
    expect(resA[0][1]).toEqual({ id: 'a' })
    expect(resB).not.toBeNull()
    expect(resB.length).toBe(1)
    expect(resB[0][0]).toEqual('resource')
    expect(resB[0][1]).toEqual({ id: 'b' })
  })
type HandlerParamsSet<T> = HandlerSet<T> & {
  params: Record<string, string>
}

      m[method] = { handler, possibleKeys: [], score: 0, name: this.name }
  private gHSets(
    node: Node<T>,
    method: string,
    params: Record<string, string>
  ): HandlerParamsSet<T>[] {
    const handlerSets: HandlerParamsSet<T>[] = []
      const handlerSet = (m[method] || m[METHOD_NAME_ALL]) as HandlerParamsSet<T>
        handlerSet.params = {}
    const handlerSets: HandlerParamsSet<T>[] = []
              handlerSets.push(...this.gHSets(nextNode.children['*'], method, node.params))
            handlerSets.push(...this.gHSets(nextNode, method, node.params))
              handlerSets.push(...this.gHSets(astNode, method, node.params))