CON-47: check the scheme of the url a Blossom server returns - #92
Merged
Merged
Conversation
…ape it The server answers an upload with a descriptor whose `url` was taken at its word and written straight into page content. `javascript:` and `data:` parse as perfectly valid URLs, so it is the protocol check and not the parse that rejects them. Our own renderer refuses those schemes and the server is operator-configured, so this is the second line — what is *stored* in a page should be sound for every future reader, not only for this client. The string handed on is the exact one the parser validated: `new URL` ignores leading and trailing C0 controls and spaces, so passing the unstripped string on would store `%20https://x/a%20` — a destination that decodes to a relative path with a space in it, a broken link built out of a URL that was fine. Not `trim()`, which would also strip a trailing U+00A0 that is part of the blob's name. The destination is percent-encoded where Markdown would end the link early: a bare `)` closes it, whitespace starts the optional title, `<`/`>` are the other destination form and a backslash escapes what follows. Encoded over UTF-8 bytes, because `\s` matches non-ASCII whitespace too and encoding U+2003 from its code unit yields `%2003` — `%20` followed by a literal `03`, pointing the link somewhere else entirely. The label was already guarded from the other side; both halves now happen at insertion time.
…ment-url # Conflicts: # docs/09-security-privacy.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes CON-47. From the CON-43 security audit.
The Blossom server answers an upload with a descriptor, and its
urlwas taken at its word and written straight into page content.What is in it
resolveAttachmentUrlaccepts the returned url only once it parses as an absolute http(s) URL.javascript:alert(1)anddata:text/html,…are perfectly valid URLs, so it is the protocol check and not the parse that rejects them; anything else falls back to the deterministic<server>/<sha256>, which is where the blob has to be anyway.new URLignores leading and trailing C0 controls and spaces, so passing the unstripped string on would have stored%20https://x/a%20— a destination that decodes to a relative path with a space in it. Nottrim(), which would also strip a trailing U+00A0 that is part of the blob's name.)closes it, whitespace starts the optional title,</>are the other destination form, a backslash escapes what follows. Encoded over UTF-8 bytes —\smatches non-ASCII whitespace too, and encoding U+2003 from its code unit yields%2003, i.e.%20followed by a literal03.How to test
VITE_BLOSSOM_SERVERat a stub that answers with{"url":"javascript:alert(1)"}— the inserted link is<server>/<hash>, not the returned string. Same for adata:url and for a relative one.)or a space in the path inserts a percent-encoded destination, and the link still opens.VITE_BLOSSOM_SERVERending in/produces no//in the stored url.src/nostr/blossom.test.tscovers the table.