Incremental conversion of L.Clipboard to typescript #13985
Conversation
Signed-off-by: Dennis Francis <[email protected]> Change-Id: Idcdb4e1e65796c707fdedb715d58cd50d6e05dbf
define typed base class member functions. Signed-off-by: Dennis Francis <[email protected]> Change-Id: Iaf24c74cb8a41469ae403fe2f26f78ada2754b9a
Signed-off-by: Dennis Francis <[email protected]> Change-Id: I12a40c89ed6857adda43ba330bcee514ea1716b0
Signed-off-by: Dennis Francis <[email protected]> Change-Id: I1187af813fd1ededcba43a3346f6f41ab79f32bb
Signed-off-by: Dennis Francis <[email protected]> Change-Id: Ib9fea6c66d7c8de45d327681d9302859557c8a3b
Signed-off-by: Dennis Francis <[email protected]> Change-Id: I806fbfed0d2d637ca85570ab293aa969eb9dfcd4
Signed-off-by: Dennis Francis <[email protected]> Change-Id: I11869800abfb63ddfe402ac271f09a7988052aa0
Signed-off-by: Dennis Francis <[email protected]> Change-Id: I4a5855a883e217da18acfe5bb8840a099c9af3a8
Signed-off-by: Dennis Francis <[email protected]> Change-Id: Iedad8a694b62e3b26bc601d32223af3ce21a922b
Signed-off-by: Dennis Francis <[email protected]> Change-Id: I06398740734c015908b657bf770d63f7eaa9b9a8
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| forClipboard: boolean, | ||
| progressFn: (progress: number) => number, | ||
| ): Promise<Blob> { | ||
| const request = new XMLHttpRequest(); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
General approach: Ensure that any URL derived from clipboard HTML and passed into _doAsyncDownload is strictly validated and normalized before use, with no way for an attacker-controlled value to escape the constraints. Since this project already has _isClipboardURLSafe and relies on it, the best fix is to (1) normalize the URL to an absolute “safe” form, (2) avoid reusing the possibly relative or ambiguous original string in later calls, and (3) slightly tighten and clarify the safety checks.
Best concrete fix without changing external behavior:
-
In
_isClipboardURLSafe, keep usingnew URL(inURL, window.location.href)but:- Explicitly reject
blob:anddata:and any non-HTTP(S)/file as already done. - Keep the origin equality check and the
/cool/clipboardpath prefix rule. - Return
falseon any parsing error (already present).
This part is mostly fine and we won’t change behavior significantly, just keep it as the central guard.
- Explicitly reject
-
In
_getMetaOrigin, instead of returninginURL(the raw, decoded string), construct and return the normalized absolute URL fromparsedURLinside_isClipboardURLSafe, or re-parse once we know it’s safe. Since we must not change_isClipboardURLSafe’s signature, a minimal change is:- After
this._isClipboardURLSafe(inURL)succeeds, create aURLagain frominURLwithwindow.location.hrefas base and returnsafeURL.hrefinstead of the originalinURL.
This guarantees that the value used later (asmetaand thensrc/url) is a fully qualified, canonical URL that still matches the restrictions.
- After
-
In
_dataTransferDownloadAndPasteAsync, before calling_doAsyncDownload, add a defensive check with_isClipboardURLSafe(src). This is effectively redundant with_getMetaOriginbut (a) it clearly documents the contract at the use site, and (b) if_getMetaOriginis ever changed, this still prevents unsafe URLs from being used. If the URL fails validation, log and fall back to the existing “fallback” paste path by directly callingdataTransferToDocumentFallback.
Because we are constrained to the provided snippets, we will:
- Adjust
_getMetaOrigininbrowser/src/app/ClipboardBase.tsto return a normalized absolute URL (safeURL.href) rather than the raw decoded string. - Add a validation guard to
_dataTransferDownloadAndPasteAsyncbefore calling_doAsyncDownload. If invalid, log and usefallbackHtmlviadataTransferToDocumentFallback.
No changes are needed in browser/src/map/Clipboard.js; it only passes the clipboard HTML to dataTransferToDocument.
| @@ -282,7 +282,18 @@ | ||
| return ''; | ||
| } | ||
|
|
||
| return inURL; | ||
| // Normalize to an absolute, canonical URL that still respects | ||
| // the same-origin and path constraints enforced by | ||
| // _isClipboardURLSafe, and use that for any subsequent requests. | ||
| try { | ||
| const safeURL = new URL(inURL, window.location.href); | ||
| return safeURL.href; | ||
| } catch (ex: any) { | ||
| window.app.console.log( | ||
| 'Failed to normalize URL: "' + inURL + '" as clipboard origin. Rejected!', | ||
| ); | ||
| return ''; | ||
| } | ||
| } else { | ||
| window.app.console.log('Mis-understood foreign origin: "' + meta + '"'); | ||
| } | ||
| @@ -420,6 +431,24 @@ | ||
| // FIXME: add a timestamp in the links (?) ignore old / un-responsive servers (?) | ||
| let response; | ||
| const errorMessage = _('Failed to download clipboard, please re-copy'); | ||
|
|
||
| // Defensive check: ensure that the source URL is still considered safe | ||
| // before performing any network request. If not, fall back to the local | ||
| // clipboard contents to avoid using an attacker-controlled URL. | ||
| if (!this._isClipboardURLSafe(src)) { | ||
| window.app.console.log( | ||
| 'Untrusted URL: "' + src + '" when downloading clipboard. Falling back to local content.', | ||
| ); | ||
| this.dataTransferToDocumentFallback( | ||
| // No DataTransfer available here, so we pass an empty object and rely | ||
| // on the HTML fallback content. | ||
| {} as DataTransfer, | ||
| fallbackHtml, | ||
| false, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| response = await this._doAsyncDownload( | ||
| 'GET', |
| meta.indexOf('%26Tag%3D') > 0 | ||
| ) { | ||
| const inURL = decodeURIComponent(meta); | ||
| if (!this._isClipboardURLSafe(inURL)) { |
There was a problem hiding this comment.
_isClipboardURLSafe() is a function I added to deal with the copilot error, but it seems not enough.
1 participant
Summary
TODO
Checklist
make prettier-writeand formatted the code.make checkmake runand manually verified that everything looks okay