Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Carrot Disclosure: Forgejo

dustri.org

Post reply on HN
Print view View original

31–40 of 60 posts

Re: Carrot Disclosure: Forgejo

#31

Post by shimman »

Chris2048 wrote: post #18
shimman wrote: post #5

Earlier quoted context omitted.

Seriously, this author comes across as an absolute sore loser if this is the PR they are referring too: https://codeberg.org/forgejo/forgejo/pulls/12283 Someone asking you to write a test for new code and then making this blog in response is just so pathetic.

> Someone asking you to write a test for new code per the response: "I'm not sure what kind of test would you like me to write for this change, as it's simply adding 4 quotes"

That totally justifies the very normal extortion like blog post in response.

Re: Carrot Disclosure: Forgejo

#32

Post by kstrauser »

Chris2048 wrote: post #18
shimman wrote: post #5

Earlier quoted context omitted.

Seriously, this author comes across as an absolute sore loser if this is the PR they are referring too: https://codeberg.org/forgejo/forgejo/pulls/12283 Someone asking you to write a test for new code and then making this blog in response is just so pathetic.

> Someone asking you to write a test for new code per the response: "I'm not sure what kind of test would you like me to write for this change, as it's simply adding 4 quotes"

Maybe one showing that the change doesn't make it worse. Here's the code change:

  - 
  +
I know zero about this code path, but suppose it's expected that `${$(this).data('href')}` is already a properly quoted value, like `"https://example.com"`. Then the first line expands to:
and the second expands to:
which would have all kinds of room for mischief. Or suppose the template engine auto-quotes values that it injects, so the quotes aren't necessary at all, which is a pretty common approach. The point is that you don't randomly want to throw quotes into HTML or single quotes into SQL just for giggles. You have to write tests demonstrating that the existing common use cases still work after the change, even if it's simply adding 4 quotes.

Re: Carrot Disclosure: Forgejo

#33

Post by bogwog »

jorams wrote: post #15

This is a weird post to be honest. You've found a whole bunch of serious security issues, filed two PRs, one of which is adding some quotes because > Those aren't exploitable XSS, but it doesn't hurt to have a second layer of defense. The other suggests breaking clients that aren't using the more secure version of an OAuth method because > I can't think of any OAuth client that would like to [use it] That second one…

And attempting to publicly shame them into accepting a PR. Kinda reminds me of https://en.wikipedia.org/wiki/XZ_Utils_backdoor

Re: Carrot Disclosure: Forgejo

#34

Post by MajesticHobo2 »

kstrauser wrote: post #32
Chris2048 wrote: post #18

Earlier quoted context omitted.

> Someone asking you to write a test for new code per the response: "I'm not sure what kind of test would you like me to write for this change, as it's simply adding 4 quotes"

Maybe one showing that the change doesn't make it worse. Here's the code change: - + I know zero about this code path, but suppose it's expected that `${$(this).data('href')}` is already a properly quoted value, like `" https://example.com "`. Then the first line expands to: and the second expands to: which would have all kinds of room for mischief. Or suppose the template engine auto-quotes values that it injects, s…

I'd say also add a test that shows the HTML injection (which spurred the PR) isn't possible. Given an attacker-controlled URL of:

    foo onclick
the following shouldn't render:
The following should:

Re: Carrot Disclosure: Forgejo

#35

Post by dangus »

JuniperMesos wrote: post #16
dangus wrote: post #7

Earlier quoted context omitted.

It would definitely be a bit silly for the author to make a fake carrot disclosure, but I thought of it just because of how reading this article made me feel distrust toward the author. IDK, they just seem like kind of a jerk! Now, I don't think the PRs with the Forgejo folks show a lot of warm collaborative energy on their side, either, but I can see how soft skills from the author would likely have taken their PRs…

> But the author's whole attitude is that Forejo is such a mess and it's barely worth their time to try and clean it up. Nobody's twisting their arm to contribute to an open source project that they don't even like! > From the perspective of Forgejo maintainers, the author is just some random new contributor barging in and telling them to drop some legacy support that hasn't been discussed in detail yet. And of cours…

The author of this blog post essentially never reported the exploit to the Forgejo maintainers. They merely submitted a security-related PR.

The maintainers aren't mind readers. They have never been directly informed that a proven exploit exists, and the author of the article actively ignored the project's reporting process despite being aware of it.

And it's not a particularly complicated report process. You literally just email them.

Re: Carrot Disclosure: Forgejo

#36

Post by unethical_ban »

PunchyHamster wrote: post #24
jorams wrote: post #15

This is a weird post to be honest. You've found a whole bunch of serious security issues, filed two PRs, one of which is adding some quotes because > Those aren't exploitable XSS, but it doesn't hurt to have a second layer of defense. The other suggests breaking clients that aren't using the more secure version of an OAuth method because > I can't think of any OAuth client that would like to [use it] That second one…

> That second one is a good idea, but the maintainer is also right to ask for some discussion before introducing a breaking change. The discussion seems to be already happening https://codeberg.org/forgejo/forgejo/issues/8634 , author of the blog just did drive-by PR rather than looking at issue tracker It's very much "I know better, do what I told you despise not thinking a second about any second order effects the…

Yeah, ITOps and software teams are totally aware of the second order effects of their shitty software and compliance failures, security are always the wrong ones.

Re: Carrot Disclosure: Forgejo

#38

Post by akoboldfrying »

onedognight wrote: post #13
shimman wrote: post #5

Earlier quoted context omitted.

Seriously, this author comes across as an absolute sore loser if this is the PR they are referring too: https://codeberg.org/forgejo/forgejo/pulls/12283 Someone asking you to write a test for new code and then making this blog in response is just so pathetic.

To hell with writing a test for you. That’s what you say to someone who gets paid by you. If the project doesn’t want the fix. That’s their issue, not the reporter’s.

Look at the big picture. The maintainers likely deal with many low-quality bug reports and PRs coming in, especially from AI, and the incentive to spam these is not going away anytime soon. How should they best allocate their limited attention?

One way is for the PR maker to signal their own attention to detail/effort/commitment by jumping through the (quite reasonable) hoop of writing a test.

Is this extra effort? Yes. But if your motivation in opening the PR in the first place is genuinely to improve the world, then do the slightly harder thing that actually improves the world given the constraints on maintainer attention it operates under, not the thing that is slightly easier for you but leaves your contribution indistinguishable from the sea of slop out there.

Re: Carrot Disclosure: Forgejo

#39

Post by kasdklasmdads »

Imagine if every open source contributor behaved like that.

"I found performance problems in your software, but I won't disclose them until you fix them."

"I'm a designer, but I won't disclose my improvement to your project until you adjust all the CSS bugs in your project."

If that person is skilled with finding security bugs, then that could be their contribution to that open-source project, like any other contribution.

Re: Carrot Disclosure: Forgejo

#40

Post by kstrauser »

MajesticHobo2 wrote: post #34
kstrauser wrote: post #32

Earlier quoted context omitted.

Maybe one showing that the change doesn't make it worse. Here's the code change: - + I know zero about this code path, but suppose it's expected that `${$(this).data('href')}` is already a properly quoted value, like `" https://example.com "`. Then the first line expands to: and the second expands to: which would have all kinds of room for mischief. Or suppose the template engine auto-quotes values that it injects, s…

I'd say also add a test that shows the HTML injection (which spurred the PR) isn't possible. Given an attacker-controlled URL of: foo onclick the following shouldn't render: The following should:

Oh, for sure! That'd end the conversation: "your change breaks the existing tests. Fix that and we'll re-consider."
Post reply on HN