loopbackio / loopbackio/loopback-next

Query parameter array parsing broken for >20 items after qs upgrade (CVE fix side effect)

オープン
#11,396 コメント 2 件 リアクション 1 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

bug help wanted
主要言語
TypeScript
スター
5.1k
フォーク
1.1k
平均マージ
2日 21時間
マージ済み PR(30日)
27

説明

Describe the bug

Current Behavior
After the qs upgrade in commit https://github.com/loopbackio/loopback-next/commit/1eedfd5057c05ac683a9c9b71c3086b61267ab81 (to address https://github.com/advisories/GHSA-6rw7-vpxm-498p), query parameter arrays with more than 20 items are being converted to objects with numeric keys instead of arrays. This causes LoopBack's parameter validation to fail.

Expected Behavior
Query parameters in the format ?ids=1&ids=2&ids=3...&ids=25 should be parsed as an array regardless of the number of items (within reasonable limits).

Steps to Reproduce

Create an endpoint with an array parameter:

typescript@get('/test')
async test(
  @param.array('ids', 'query', { type: 'string' })
  ids: string[]
): Promise<any> {
  return { ids };
}
  1. Make a request with 21+ repeated query parameters:

GET /test?ids=1&ids=2&ids=3...&ids=21&ids=22

Observe the validation error:

json{
  "error": {
    "statusCode": 400,
    "name": "BadRequestError",
    "message": "Invalid data [{\"0\":\"1\",\"1\":\"2\",...}] for parameter \"ids\".",
    "code": "INVALID_PARAMETER_VALUE"
  }
}

Root Cause

The qs library by default limits array indices to 20 (https://github.com/ljharb/qs#parsing-arrays). When more than 20 items are provided, it converts the array to an object with numeric keys to prevent DoS attacks with extremely large indices like a[999999999].

This is documented behavior in the qs library, and there is an ongoing discussion about this limitation, since it was introduced in a patch version instead of a breaking change version: https://github.com/ljharb/qs/issues/537

Impact
This breaks existing APIs that accept more than 20 array items via query parameters.

Proposed Solution
Configure qs with a higher arrayLimit option. The default of 20 is too restrictive.

I encourage the LoopBack team to contribute to the discussion in the qs issue thread (https://github.com/ljharb/qs/issues/537) to help get this resolved at the library level, and in the meantime provide a way for LoopBack applications to configure this limit.

Suggested approaches for LoopBack:

Make arrayLimit configurable via RestServer options so applications can set their own limits
Increase the default arrayLimit to a more reasonable value (e.g., 100 or 1000)
Document the workaround for applications that need to handle this


LoopBack version: 4.x (any version after the qs upgrade)
Node.js version: [applicable to all]
Operating system: [applicable to all]

Additional Context

Related CVE fix: https://github.com/advisories/GHSA-6rw7-vpxm-498p
qs documentation: https://github.com/ljharb/qs#parsing-arrays
Ongoing discussion in qs repo: https://github.com/ljharb/qs/issues/537

Logs

Additional information

No response

Reproduction

https://github.com/ljharb/qs/issues/537

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

まず、REST サーバーのオプションを通じたクエリ解析と、issue で説明されている qs arrayLimit の動作を追跡します。設定可能な上限のアプローチと、より高いデフォルト値のアプローチを比較し、そのうえで、20 を超える繰り返しクエリパラメーターが配列のまま維持され、DoS 保護を削除せずにパラメーター検証を通過することを確認します。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
typescript
領域
api, backend
issue の種類
バグ
難易度
3/5
見積もり時間
1〜2日
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
45/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。