fix(bearer-auth): escape regex metacharacters in bearer auth prefix option#4750
Conversation
EdamAme-x
left a comment
There was a problem hiding this comment.
@otoneko1102 Can you check this?
fix: review
for nodejs 22
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4750 +/- ##
==========================================
+ Coverage 91.48% 92.83% +1.35%
==========================================
Files 177 177
Lines 11556 11642 +86
Branches 3357 3466 +109
==========================================
+ Hits 10572 10808 +236
+ Misses 983 833 -150
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey @otoneko1102 Thank you for the PR. I realized you don't need to use a regexp match for the prefix value. If so, the diff --git a/src/middleware/bearer-auth/index.ts b/src/middleware/bearer-auth/index.ts
index 3b9d225f..1a7e7854 100644
--- a/src/middleware/bearer-auth/index.ts
+++ b/src/middleware/bearer-auth/index.ts
@@ -13,7 +13,6 @@ const TOKEN_STRINGS = '[A-Za-z0-9._~+/-]+=*'
const PREFIX = 'Bearer'
const HEADER = 'Authorization'
-const escapeRegExp = (str: string): string => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
type MessageFunction = (c: Context) => string | object | Promise<string | object>
type CustomizedErrorResponseOptions = {
wwwAuthenticateHeader?: string | object | MessageFunction
@@ -113,9 +112,9 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
}
const realm = options.realm?.replace(/"/g, '\\"')
- const prefixRegexStr = options.prefix === '' ? '' : `${escapeRegExp(options.prefix)} +`
- const regexp = new RegExp(`^${prefixRegexStr}(${TOKEN_STRINGS}) *$`, 'i')
- const wwwAuthenticatePrefix = options.prefix === '' ? '' : `${options.prefix} `
+ const prefix = options.prefix
+ const tokenRegexp = new RegExp(`^${TOKEN_STRINGS}$`)
+ const wwwAuthenticatePrefix = prefix === '' ? '' : `${prefix} `
const throwHTTPException = async (
c: Context,
@@ -165,8 +164,19 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
'Unauthorized'
)
} else {
- const match = regexp.exec(headerToken)
- if (!match) {
+ let tokenValue: string | undefined
+
+ if (prefix === '') {
+ tokenValue = headerToken
+ } else {
+ const headerLower = headerToken.toLowerCase()
+ const prefixLower = prefix.toLowerCase()
+ if (headerLower.startsWith(prefixLower) && headerToken[prefix.length] === ' ') {
+ tokenValue = headerToken.slice(prefix.length).trimStart()
+ }
+ }
+
+ if (!tokenValue || !tokenRegexp.test(tokenValue)) {
// Invalid Request
await throwHTTPException(
c,
@@ -180,12 +190,12 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
} else {
let equal = false
if ('verifyToken' in options) {
- equal = await options.verifyToken(match[1], c)
+ equal = await options.verifyToken(tokenValue, c)
} else if (typeof options.token === 'string') {
- equal = await timingSafeEqual(options.token, match[1], options.hashFunction)
+ equal = await timingSafeEqual(options.token, tokenValue, options.hashFunction)
} else if (Array.isArray(options.token) && options.token.length > 0) {
for (const token of options.token) {
- if (await timingSafeEqual(token, match[1], options.hashFunction)) {
+ if (await timingSafeEqual(token, tokenValue, options.hashFunction)) {
equal = true
break
}@otoneko1102 @EdamAme-x @usualoma What do you think of this? |
|
I agree with you |
|
@otoneko1102 Thanks! Can you change the code? |
|
@yusukebe OK! I'll do it later. |
|
@otoneko1102 Thanks. Please ping me if it's done! |
|
@yusukebe Sorry for the delay! Fixed it! An error has occurred for Node.js v20 test. |
|
Looks good! |
|
The CI works well now. Looks good! Thanks! |
The author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the code"prefix" option in Bearer Auth middleware is interpolated directly into
new RegExp()without escaping. This may cause SyntaxError crashes or unintended pattern matching when "prefix" contains regex metacharacters.e.g., (, ), ., +, ...
prefix: 'Bearer|Token'...