4 ms·

The author's attitude is so off-putting. What gives? Did Forgejo hurt you? The Forgejo disclosure process looked pretty simple and straightforward to me. The b

by dangus 6mo ago

The author's attitude is so off-putting. What gives? Did Forgejo hurt you? The Forgejo disclosure process looked pretty simple and straightforward to me. The bold and all-caps words that bothered the author are just making sure you know how to disclose vulnerabilities safely without leaking zero-day exploits to a wider audience than necessary. I'm also not impressed with a carrot disclosure that looks like this. Running a python script to compromise a locally hosted instance? Bruh, you have physical hardware and host shell access. That python script could be doing anything including running as root. Show us the exploit hitting a remote server.

shimman 6mo ago
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 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.
- onedognight 6mo ago
  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.
  - akoboldfrying 6mo ago
    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.
- martey 6mo ago
  While I agree with you that this blog post (and the "carrot disclosure" described in it) is ill-considered, the pull request is not really "new code", it adds quotes to HTML attributes that are missing them. I think it's entirely reasonable for a contributor to assume that a new test case would not be needed for this small change, and that the maintainer's response ("So a simple question: is this code covered under a test? If not, you will have to add one.") is more abrasive than necessary.
  - preisschild 6mo ago
    a test is probably not the right thing for this, but adding a linting rule so that quoting is enforced everywhere is probably the right way to go
- Chris2048 6mo ago
  > 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"
  - shimman 6mo ago
    That totally justifies the very normal extortion like blog post in response.
  - kstrauser 6mo ago
    Maybe one showing that the change doesn't make it worse. Here's the code change: - <a class="item muted sidebar-item-link" href=${$(this).data('href')}> + <a class="item muted sidebar-item-link" href="${$(this).data('href')}"> 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 https://example.com"`. Then the first line expands to: <a class="item muted sidebar-item-link" href="https://example.com"> and the second expands to: <a class="item muted sidebar-item-link" href=""https://example.com""> 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.
    - MajesticHobo2 6mo ago
      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: <a class="item muted sidebar-item-link" href=foo onclick> The following should: <a class="item muted sidebar-item-link" href="foo onclick">
      - kstrauser 6mo ago
        Oh, for sure! That'd end the conversation: "your change breaks the existing tests. Fix that and we'll re-consider."
quectophoton 6mo ago
> I'm also not impressed with a carrot disclosure that looks like this. Running a python script to compromise a locally hosted instance? Bruh, you have physical hardware and host shell access. That python script could be doing anything including running as root. > Show us the exploit hitting a remote server. Watch out, their script works on HN too, as a proof here's me logging in to YOUR computer's root account (a bit more redacted for obvious reasons): $ python3 ./poc/chain_alpha.py --target dangus > out.txt $ grep Backdoor out.txt | sed -r 's@[^:]+$@ [REDACTED]@g' [+] Backdoor admin created: [REDACTED] $ grep IP out.txt | sed -r 's@[^:]+$@ [REDACTED]@g' [+] IPv4 address for dangus: [REDACTED] $ grep 'debug2: shell' out.txt [+] debug2: shell request accepted on channel 0 $ tail -n12 out.txt ================================================================ [+] COMMAND EXECUTION CONFIRMED! ================================================================ Server-side output (received via SSH, with `set -x`): + id -u 0 + id -g 0 ================================================================ $ sha256 ./poc/chain_alpha.py c10d28a5ff74646683953874b035ca6ba56742db2f95198b54e561523e1880d7 ./poc/chain_alpha.py

by dangus 6mo ago

shimman 6mo ago

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 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.

onedognight 6mo ago
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.
- akoboldfrying 6mo ago
  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.
martey 6mo ago
While I agree with you that this blog post (and the "carrot disclosure" described in it) is ill-considered, the pull request is not really "new code", it adds quotes to HTML attributes that are missing them. I think it's entirely reasonable for a contributor to assume that a new test case would not be needed for this small change, and that the maintainer's response ("So a simple question: is this code covered under a test? If not, you will have to add one.") is more abrasive than necessary.
- preisschild 6mo ago
  a test is probably not the right thing for this, but adding a linting rule so that quoting is enforced everywhere is probably the right way to go
Chris2048 6mo ago
> 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"
- shimman 6mo ago
  That totally justifies the very normal extortion like blog post in response.
- kstrauser 6mo ago
  Maybe one showing that the change doesn't make it worse. Here's the code change: - <a class="item muted sidebar-item-link" href=${$(this).data('href')}> + <a class="item muted sidebar-item-link" href="${$(this).data('href')}"> 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 https://example.com"`. Then the first line expands to: <a class="item muted sidebar-item-link" href="https://example.com"> and the second expands to: <a class="item muted sidebar-item-link" href=""https://example.com""> 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.
  - MajesticHobo2 6mo ago
    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: <a class="item muted sidebar-item-link" href=foo onclick> The following should: <a class="item muted sidebar-item-link" href="foo onclick">
    - kstrauser 6mo ago
      Oh, for sure! That'd end the conversation: "your change breaks the existing tests. Fix that and we'll re-consider."

quectophoton 6mo ago

> I'm also not impressed with a carrot disclosure that looks like this. Running a python script to compromise a locally hosted instance? Bruh, you have physical hardware and host shell access. That python script could be doing anything including running as root. > Show us the exploit hitting a remote server. Watch out, their script works on HN too, as a proof here's me logging in to YOUR computer's root account (a bit more redacted for obvious reasons): $ python3 ./poc/chain_alpha.py --target dangus > out.txt $ grep Backdoor out.txt | sed -r 's@[^:]+$@ [REDACTED]@g' [+] Backdoor admin created: [REDACTED] $ grep IP out.txt | sed -r 's@[^:]+$@ [REDACTED]@g' [+] IPv4 address for dangus: [REDACTED] $ grep 'debug2: shell' out.txt [+] debug2: shell request accepted on channel 0 $ tail -n12 out.txt ================================================================ [+] COMMAND EXECUTION CONFIRMED! ================================================================ Server-side output (received via SSH, with `set -x`): + id -u 0 + id -g 0 ================================================================ $ sha256 ./poc/chain_alpha.py c10d28a5ff74646683953874b035ca6ba56742db2f95198b54e561523e1880d7 ./poc/chain_alpha.py