starred/audiobookshelf
1
0
Fork 0
mirror of https://github.com/advplyr/audiobookshelf.git synced 2026-05-30 23:40:40 +02:00
Code Issues 1.0k Packages Projects Releases 10 Wiki Activity

[PR #3963] [MERGED] Security fix for GHSA-pg8v-5jcv-wrvw #4134

New Issue
Closed
opened 2026-04-25 00:18:27 +02:00 by adam · 0 comments
adam commented 2026-04-25 00:18:27 +02:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf/pull/3963
Author: @mikiher
Created: 2/11/2025
Status: ✅ Merged
Merged: 2/11/2025
Merged by: @advplyr

Base: master ← Head: security-fix-GHSA-pg8v-5jcv-wrvw

📝 Commits (1)

  • ec65376 Security fix for GHSA-pg8v-5jcv-wrvw

📊 Changes

1 file changed (+7 additions, -2 deletions)

View changed files

📝 server/Auth.js (+7 -2)

📄 Description

Brief summary

This fixes the security issue noted in the advisory below:

  • it matches against anchored patterns
  • it tests req.path against the patterns (instead of req.originalUrl)

Which issue is fixed?

https://github.com/advplyr/audiobookshelf/security/advisories/GHSA-pg8v-5jcv-wrvw

In-depth Description

Because we are now testing req.path against anchored patterns, the pattens needed to be changed a bit, since this code (which is called from the ifAuthNeeded middleware is used in two separate locations:

  • at the app level, where req.path begins with ${global.RouterBasePath}/api
  • at the API router level, where req.path is already stripped of ${global.RouterBasePath}/api

So the patterns need to change to:

      new RegExp(`^(${escapedRouterBasePath}/api)?/items/[^/]+/cover$`), 
      new RegExp(`^(${escapedRouterBasePath}/api)?/authors/[^/]+/image$`)

In our case escapedRouterBasePath will be /audiobookshelf, so the patterns will accept the following examples:

/audiobookshelf/api/items/{itemid}/cover
/audiobookshelf/api/authors/{authorid}/image
/items/{itemid}/cover
/authors/{authorid}/image

How have you tested this?

  • Checked the patterns against all the malicious URLs mentioned in the advisory. They now match none of them.
  • Checked that the ifAuthNeeded still identifies covers and author image requests as not requiring authentication, in both locations used.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf/pull/3963 **Author:** [@mikiher](https://github.com/mikiher) **Created:** 2/11/2025 **Status:** ✅ Merged **Merged:** 2/11/2025 **Merged by:** [@advplyr](https://github.com/advplyr) **Base:** `master` ← **Head:** `security-fix-GHSA-pg8v-5jcv-wrvw` --- ### 📝 Commits (1) - [`ec65376`](https://github.com/advplyr/audiobookshelf/commit/ec6537656925a43871b07cfee12c9f383844d224) Security fix for GHSA-pg8v-5jcv-wrvw ### 📊 Changes **1 file changed** (+7 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `server/Auth.js` (+7 -2) </details> ### 📄 Description ## Brief summary This fixes the security issue noted in the advisory below: - it matches against anchored patterns - it tests `req.path` against the patterns (instead of `req.originalUrl`) ## Which issue is fixed? https://github.com/advplyr/audiobookshelf/security/advisories/GHSA-pg8v-5jcv-wrvw ## In-depth Description Because we are now testing `req.path` against anchored patterns, the pattens needed to be changed a bit, since this code (which is called from the `ifAuthNeeded` middleware is used in two separate locations: - at the app level, where `req.path` begins with `${global.RouterBasePath}/api` - at the API router level, where `req.path` is already stripped of `${global.RouterBasePath}/api` So the patterns need to change to: ```js new RegExp(`^(${escapedRouterBasePath}/api)?/items/[^/]+/cover$`), new RegExp(`^(${escapedRouterBasePath}/api)?/authors/[^/]+/image$`) ``` In our case escapedRouterBasePath will be `/audiobookshelf`, so the patterns will accept the following examples: ``` /audiobookshelf/api/items/{itemid}/cover /audiobookshelf/api/authors/{authorid}/image /items/{itemid}/cover /authors/{authorid}/image ``` ## How have you tested this? - Checked the patterns against all the malicious URLs mentioned in the advisory. They now match none of them. - Checked that the ifAuthNeeded still identifies covers and author image requests as not requiring authentication, in both locations used. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2026-04-25 00:18:27 +02:00
adam closed this issue 2026-04-25 00:18:27 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
authentication
backlog
bug
chapter editor
config-issue
ebooks
encoding/embedding
enhancement
help wanted
listening sessions & progress
planned
possible plugin
progress sync
pull-request
Mirrored from GitHub Pull Request
 sorting/filtering/searching
unable to reproduce
upload
users & permissions
waiting
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/audiobookshelf#4134
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?