From ddb878d9854abcde3adcb00f04a467655ff71db1 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Thu, 4 Jun 2026 10:28:33 -0400 Subject: [PATCH] Fix ACL case sensitivity issues in sqlite --- docs/releases.md | 4 ++++ user/manager_sqlite.go | 8 ++++++- user/manager_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/releases.md b/docs/releases.md index 5ed4bdf9..3d82c1e2 100644 --- a/docs/releases.md +++ b/docs/releases.md @@ -1926,6 +1926,10 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release ### ntfy server v2.24.0 (UNRELEASED) +**Security issues:** + +* Fix case-insensitive ACL topic matching on SQLite: an access control rule for `secret` no longer also matches a request for `SECRET`. SQLite's `LIKE` is case-insensitive for ASCII by default; PostgreSQL was unaffected (no ticket) +* **Features:** * Add opt-in in-memory ACL cache (`auth-access-cache`) that serves topic authorization without a database round-trip; off by default, intended for high-volume servers diff --git a/user/manager_sqlite.go b/user/manager_sqlite.go index 18cb4028..62652ce3 100644 --- a/user/manager_sqlite.go +++ b/user/manager_sqlite.go @@ -312,7 +312,13 @@ func NewSQLiteManager(filename, startupQueries string, config *Config) (*Manager if !util.FileExists(parentDir) { return nil, fmt.Errorf("user database directory %s does not exist or is not accessible", parentDir) } - d, err := sql.Open("sqlite3", filename) + // Open with case-sensitive LIKE. ACL topic matching is done via LIKE (see + // selectTopicPerms), and SQLite's LIKE is case-insensitive for ASCII by + // default -- without this, an ACL rule for "secret" would also match a + // request for "SECRET", which is a security iisue. PostgreSQL's LIKE is + // already case-sensitive, so this only affects SQLite. The pragma is + // applied to every pooled connection by the driver. + d, err := sql.Open("sqlite3", fmt.Sprintf("%s?_case_sensitive_like=on", filename)) if err != nil { return nil, err } diff --git a/user/manager_test.go b/user/manager_test.go index a3fd07b1..6d13929e 100644 --- a/user/manager_test.go +++ b/user/manager_test.go @@ -2414,6 +2414,55 @@ func TestAccessCache_FullReloadDoesNotClobberConcurrentRevoke(t *testing.T) { }) } +// TestAuthorizeTopicAccess_TopicMatchingIsCaseSensitive guards against ACL +// topic matching being case-insensitive. SQLite's LIKE is case-insensitive for +// ASCII by default, which would let a request for "SECRET" match an ACL rule +// for "secret" -- a security hole. PostgreSQL's LIKE is already case-sensitive. +// NewSQLiteManager opens the database with case_sensitive_like enabled to close +// this gap. This exercises the direct-DB path (cache disabled), which is the +// path that runs the LIKE query; the in-memory cache is independently +// case-sensitive (Go map keys / case-sensitive regex). +func TestAuthorizeTopicAccess_TopicMatchingIsCaseSensitive(t *testing.T) { + forEachBackend(t, func(t *testing.T, newManager newManagerFunc) { + a := newManager(&Config{ + DefaultAccess: PermissionDenyAll, + BcryptCost: bcrypt.MinCost, + AccessCacheEnabled: false, // exercise the direct-DB LIKE path + }) + t.Cleanup(func() { a.Close() }) + + require.Nil(t, a.AddUser("ben", "mypass", RoleUser, false)) + require.Nil(t, a.AllowAccess("ben", "secret", PermissionReadWrite)) // exact rule + require.Nil(t, a.AllowAccess("ben", "team*", PermissionReadWrite)) // wildcard rule, stored as "team%" + + // The exact rule is honored verbatim. + read, write, found, err := a.authorizeTopicAccess("ben", "secret") + require.Nil(t, err) + require.True(t, found) + require.True(t, read) + require.True(t, write) + + // Case variants of the exact rule must NOT match. + for _, topic := range []string{"SECRET", "Secret", "sEcReT"} { + _, _, found, err := a.authorizeTopicAccess("ben", topic) + require.Nil(t, err) + require.False(t, found, "ACL rule for \"secret\" must not match %q (case-insensitive match is a security hole)", topic) + } + + // The wildcard rule is honored for the matching case. + _, _, found, err = a.authorizeTopicAccess("ben", "team-rocket") + require.Nil(t, err) + require.True(t, found) + + // Case variants of the wildcard prefix must NOT match. + for _, topic := range []string{"TEAM-rocket", "Team-rocket", "TEAMING"} { + _, _, found, err := a.authorizeTopicAccess("ben", topic) + require.Nil(t, err) + require.False(t, found, "wildcard rule for \"team*\" must not match %q", topic) + } + }) +} + func TestStoreReservations(t *testing.T) { forEachStoreBackend(t, func(t *testing.T, manager *Manager) { require.Nil(t, manager.AddUser("phil", "mypass", RoleUser, false))