Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It's still possible to disguise links as other links #1397

Closed
ekzyis opened this issue Sep 12, 2024 · 1 comment
Closed

It's still possible to disguise links as other links #1397

ekzyis opened this issue Sep 12, 2024 · 1 comment

Comments

@ekzyis
Copy link
Member

ekzyis commented Sep 12, 2024

Description

This code exists to avoid that one can write stacker.news which looks like it's a link to stacker.news but it's actually not (this works on Github too though).

// If [text](url) was parsed as <a> and text is not empty and not a link itself,
// we don't render it as an image since it was probably a conscious choice to include text.
const text = children[0]
let url
try {
url = !href.startsWith('/') && new URL(href)
} catch {
// ignore invalid URLs
}

The above was wrong, it's actually the isRawURL check in this code that was written to avoid disguised links.

However, it only works for
[https://stacker.news](https://www.youtube.com/watch?v=dQw4w9WgXcQ)]
and not
[stacker.news](https://www.youtube.com/watch?v=dQw4w9WgXcQ).

function LinkRaw ({ href, children, src, rel }) {
const isRawURL = /^https?:\/\//.test(children?.[0])
return (
// eslint-disable-next-line
<a
target='_blank'
rel={rel ?? UNKNOWN_LINK_REL}
href={src}
>{isRawURL || !children ? src : children}
</a>
)
}

It doesn't even get hit for stacker.news as text.

Steps to Reproduce

  1. Write [stacker.news](https://www.youtube.com/watch?v=dQw4w9WgXcQ)
  2. Click preview
  3. See that it gets rendered as a link to stacker.news but it's a link to youtube.com

Expected behavior

It should refuse to use the text chosen by the user if it looks like it's pretending to be another link

Logs
If applicable, add your browsers console logs.

Environment:
If you only experience the issue on certain devices or browsers, provide that info.

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS]
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Additional context

@tsmith123
Copy link
Contributor

tsmith123 commented Sep 24, 2024

I've gone ahead and attempted to fix this - not sure if you decided on a "wontfix"???

#1428

huumn added a commit that referenced this issue Sep 28, 2024
@huumn huumn closed this as completed in cc4bbf9 Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants