Skip to content

Commit

Permalink
limit TitleExtractor to allow only Remark42 whitelisted domains
Browse files Browse the repository at this point in the history
Allowed domains consist of `REMARK_URL` second-level domain (or whole IP in case it's IP like `127.0.0.1`) and `ALLOWED_HOSTS`. That is needed to prevent Remark42 from asking arbitrary servers and storing the page title as the comment.PostTitle.

Previous behaviour allowed the caller of the API to create a comment
with an arbitrary URL and learn the title of the page, which might be
accessible to the server Remark42 is installed on but not to the user
outside that network (CWE-918).
  • Loading branch information
paskal committed Oct 10, 2023
1 parent a5c4e68 commit 1f26623
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
2 changes: 0 additions & 2 deletions backend/app/cmd/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"os/signal"
"path"
"regexp"
"slices"
"strings"
"syscall"
"time"
Expand Down Expand Up @@ -662,7 +661,6 @@ func (s *ServerCommand) getAllowedDomains() []string {
}

if domain != "" && // don't add empty domain as it will allow everything to be extracted
!slices.Contains(allowedDomains, domain) && // don't duplicate domains
(domain == "localhost" || len(strings.Split(domain, ".")) > 1) { // don't allow single-word domains like "com" except localhost
allowedDomains = append(allowedDomains, domain)
}
Expand Down
4 changes: 2 additions & 2 deletions backend/app/cmd/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,8 +737,8 @@ func Test_getAllowedDomains(t *testing.T) {
// incorrect URLs, so Hostname is empty but returned list doesn't include empty string as it would allow any domain
{ServerCommand{AllowedHosts: []string{}, CommonOpts: CommonOpts{RemarkURL: "bad hostname"}}, []string{}},
{ServerCommand{AllowedHosts: []string{}, CommonOpts: CommonOpts{RemarkURL: "not_a_hostname"}}, []string{}},
// test removal of 'self', multiple AllowedHosts and deduplication
{ServerCommand{AllowedHosts: []string{"'self'", "example.org", "test.example.org", "remark42.com"}, CommonOpts: CommonOpts{RemarkURL: "https://example.org"}}, []string{"example.org", "remark42.com"}},
// test removal of 'self', multiple AllowedHosts. No deduplication is expected
{ServerCommand{AllowedHosts: []string{"'self'", "example.org", "test.example.org", "remark42.com"}, CommonOpts: CommonOpts{RemarkURL: "https://example.org"}}, []string{"example.org", "example.org", "remark42.com", "example.org"}},
}
for i, tt := range tbl {
t.Run(strconv.Itoa(i), func(t *testing.T) {
Expand Down

0 comments on commit 1f26623

Please sign in to comment.