Carrot disclosure: Forgejo
81 points by 7tehdt3cnw6kir6o
81 points by 7tehdt3cnw6kir6o
So it seems the author found an RCE among other issues, didn't follow Forgejo's requested disclosure process, sent a PR from nowhere which got closed, and is now telling everybody about the RCE without providing details to Forgejo.
didn't follow Forgejo's requested disclosure process
It's ok to call out a too convoluted process. Other companies manage to simplify this by creating a submission portal for example, making multiple points like encrypted messages moot. As a submitter, I care little about the process the company wants to follow in an ideal situation and I care a lot about "just take it and fix it".
It's on Forgejo's interest to not turn people away with a long process. We all lose if that happens.
I mean, it's not even that complicated. Took maybe a few minutes to read through and the main idea is "send an encrypted email". That's a pretty low bar to take a public disclosure stance on, and it seems more attention-seeking than actually wanting to remediate the problem IMHO.
I don’t know how to put this but their constant use of MUST and MUST NOT comes across as pretty demanding for a team who’s getting free support from a community member after said team fucked up and shipped a security vulnerability. Like… I appreciate where their head is at but… give me an email address I can report it to, without having to like… figure out how to use gpg with Apple Mail, and go ahead and request that I not share the details with anyone until it’s been patched. Don’t tell me what I MUST or MUST NOT do, and don’t assume you have the authority to make such demands.
I think I’m with the author on this one. That disclosure process is grating to me.
That language was almost certainly taken from IETF BCP 14 (RFC 2119 and 8174), where those words have specific meanings, but only if in all capitals. The meaning is lost somewhat when using it outside of IETF context without a reference.
That might make it seem harsher than intended by the authors, which is on them. And even considering that, this policy looks exhausting.
Definitely IETF language, but used incorrectly. It's intended to remove ambiguity, but here just adds it.
For example, the first two bullets are:
If you discover a security vulnerability in Forgejo, you MUST send an encrypted email to security@forgejo.org, combined with all available details. The same applies when a security issue was not properly addressed in Forgejo.
This email SHOULD be encrypted, or you should contact us first so we can provide a secure channel for the transmission of the details. You SHOULD NOT share details about the issue via insecure channels. Please pay special attention when quoting email conversations.
So MUST it be an encrypted email, or SHOULD it be an encrypted email? The email SHOULD be encrypted, or you should contact them: is the second should also a SHOULD? Is it that you SHOULD do one or the other, or you MUST do one or the other, or something else?
I mean, it's not even that complicated.
It doesn't have to be objectively complicated. It may be subjectively annoying enough to write a blog post about instead. Also the reporter may be on their third round of "no, you don't understand, it actually is an issue given (context), no sending you a screen recording won't explain it any better" with another vendor. (hi Apple...) Maybe they just happened to push someone over the threshold.
I know I've opened issues before like "this is the location of this bug, change (...) here. I've got a patch, but I'm not jumping through your hoops of signing legal stuff".
It does seem weird to request encrypted email for security issues. Not even sourcehut, which is email first (but not email only) does that. Instead they use a private mailing list instead. I would have expected that forgejo supported filing a 'private' issue instead for use cases like this one (or support issues were personal info might be involved)
They don't currently have private issues, which is something the Fedora folks are looking to get added iirc
They sent 5 more pull requests
Looking into https://codeberg.org/forgejo/forgejo/pulls/12291/ my first thought was it would be a non issue. Like an 🫏 I had assumed that oauth2.ParseToken only translates what is sent from the front-end to a domain model. So checking the type there wouldn't really matter as the code would still have to look up the refresh token in the database. And I was assuming a structure similar to.
create table oauth_tokens (
access_token text not null,
refresh_token text
)
Passing an access token as a refresh token would likely run into the token not being found. But looking into ParseToken, it turns out there is no DB involved. Apparently Forgejo uses jwt for OAuth tokens and ParseToken is also responsible for validating the token! 😮💨
That said that PR for sure needs to add a test. It seems the author didn't want to do the work and instead expected the fact that the changes were related to security to be enough to get them over them.
Probaby the validation wants to be centralized in tokens as well as checking any pertinent claims like expiration time. It might be useful to express the type of token as a claim and have ParseToken take a sequence of claims to verify as third argument, but I'm not familiar with go's jwt library.
While I like to give grace to a FOSS project, and would certainly offer my best effort at disclosing reasonably, I find this policy obnoxious. If a commercial vendor published something like that, my response might be to send my full PoC exploit to the full disclosure mailing list, and let the skids sort them out. Especially if they brushed off my attempt at a more discrete disclosure.
I absolutely recognize (and love!) that FOSS is different. But telling me what I MUST or MUST NOT do relative to a defect I found in your software rubs me the wrong way. I'd like to think I'd react better than "carrot disclosure" but I might not, on the wrong day.
I think this may be partly an issue of different cultural expectations. Signs and instructions in english have a lot more politeness words than the same signs in (say) French or German.
And, yeah, as others have said, the capitalised "MUST" is probably a good-faith attempt at being clear by using IETF terms.
The policy still isn't great, and I can see why it (and the responses to their PRs) could aggravate someone.
it's nonsense tho:
This is not how MUST works.
I think it's probably a typo from them changing their mind during drafting.
I don't think it's a good security policy, and I would also be annoyed if I pointed out really obvious security holes to people for free and then they asked me to jump thru hoops.
I just think people are getting a bit hung-up on it.
I do think the critique of the security reporting policy is valid, it's a lot to agree to or even just read.
I think it would be simpler to have an email people can submit to and maybe 2-3 more bullet points about what to expect; the rest can be spelled out in a separate document. Like, an unencrypted email with a vulnerability report (forbidden per their policy) is a lot better than whatever this has turned into.
Edit: I think the CONTRIBUTING.md in the repository itself is what I was looking/hoping for, it seems fine.
forbidden per their policy
I don’t think it actually is. The relevant part of the policy is badly written:
- If you discover a security vulnerability in Forgejo, you MUST send an encrypted email to security@forgejo.org, combined with all available details. The same applies when a security issue was not properly addressed in Forgejo.
- This email SHOULD be encrypted, or you should contact us first so we can provide a secure channel for the transmission of the details. You SHOULD NOT share details about the issue via insecure channels. Please pay special attention when quoting email conversations.
I contend that it’s linguistically more sound to interpret the MUST as compelling mandatory reporting (which I know of no legal basis for), rather than saying the email you send must be encrypted. Combine it with the second point which only says that the email SHOULD be encrypted, and this becomes an even stronger contention. https://codeberg.org/forgejo/governance/issues/159#issuecomment-2254416 (“I considered … • you MUST notify Forgejo • you SHOULD use the encrypted email address for it”) further supports this.
I agree. Maybe this will provide some motivation for a feature similar to github's "security advisories" tool, which allows a pretty streamlined "privately discuss, fix, and publish info about security vulns...".
Yeah, this has been historically pushed back in favour of deferring to other secure communication methods. I don't blame them for this, it seems to be a fundamental change of structure of issues if not a new feature entirely.
A new feature entirely makes more sense to me than trying to shoehorn it in as part of issues.
I just assumed something like that was either deemed out of scope, or deferred due to lack of time/personnel/etc.
Lack of time/personnel but also lack of definitive conclusion on how it should be implemented. I've been following the issue for a while, and it's made basically no movement because external contributors who want this feature (myself included) haven't really gotten strong feedback on how to go about it. Previous attempts have been closed for going about it the wrong way, too. I don't blame them for wanting to keep things pretty tight and reduce the number of features they have to maintain, though, especially on a wire-thin budget and for features that can be accomplished by other means.
speaking nothing about the bugs as i haven't had time to properly dig into this yet, but as someone whose entire job is more or less days quietly messaging organisations about security issues i found, i frankly don't blame them. i get paid to do this, unlike the author of the article. i took a look at the forgejo security guidelines and honestly? obnoxious and pretentious. the overwhelming majority of the clients i work with present me with far, far LESS prickly a guideline, with far fewer CAPITAL KEYWORDS pretending THAT i am an ietf STANDARD, or something similarly load-bearing. i get the point of them, i've spent hundreds of hours buried in ietf rfcs, but you "SHOULD" get over yourself here.
ultimately security reports like this are submitted to you in the same vein of good graces that open source software is provided: as is, with no warranties. if you value these contributions, you must ensure that the friction is minimal. these people have already spent time spelunking the software, identifying genuine defects, and having the courtesy to tell you instead of selling it on the market (for what it's worth, for those interested, the market for forgejo vulnerabilities is presently nil, but still). if you're going to make it a massive hassle, they simply won't bother. honestly looking at forgejo's, i probably wouldn't either, but i'd simply opt to not bother posting about it at all.
how many others have been found by individuals that look at the requirements for reporting these responsibly and then decided "nah"? this is curl's. you might notice it is quite long, just like forgejo's. however, curl's covers far, far more than just "how to report a vulnerability". it covers entire sections like "what comes next". the actual "process" documented for curl is:
that's it.
if it's good enough for curl, it's good enough for you. i think the current policy is simply taking itself far too seriously for reality, and it's a genuine handicap.
frankly this makes me slightly concerned about my usage of forgejo. too busy taking the wrong things seriously.
I guess I am glad that the code is at least auditable at all, and not some private SaaS/Vendor code.
But given the sorry state of the codebase, I'm pretty sure I could spend another evening and find another chain.
It would be interesting to know how much of the questionable code is inherited from gitea vs originating in the forgejo fork.
For what it’s worth you kinda can audit GitHub source code. At least version of it. It’s not official but you can obtain GitHub EE images and they contain the code. It’s lightly obfuscated but it’s easy to undo and you get the complete original source code. It’s not fully identical to the hosted app but I imagine it’s not vastly different.
The GitHub EE code is getting less readable. The recent Wiz / GitHub vulnerability report says they had to decompile several binaries to trace how git push options are used on the server. And GitHub’s (lack of) availability blog post says they are replacing Ruby with Golang.
It seems that the author's posts on various Mastodon instances are getting removed.
I am not part of those Mastodon instance moderation teams and they are free to apply their own terms on their own instances, but this sounds like abuse to me according to their public rules:
So, for what reason are they protecting Forgejo/Codeberg?
i didn't notice this before, that is extremely weird. the reality is though that those instances don't need any reason to remove the content beyond "i don't want this on the instance i host", where this is, well, literally anything.
This is one of the issues the author found: https://codeberg.org/jvoisin/forgejo/commit/1ed43c23e66db555a3dceeff05875a3334ed7921
The Forgejo team work incredibly hard. I'm a new Forgejo contributor myself and have found them to be endlessly patient with me as I've learned how to become a more helpful member of the project.
The email-based security policy is very clear, but I could understand missing that it existed. I missed a few things from Forgejo's new contributor docs in my first few PRs too. People pointed them out to me and I gratefully read up and amended my approach.
I would hope anyone thinking of becoming a Forgejo contributor would handle that kind of feedback constructively, with a view to building a productive working relationship with the core maintainer team. Even when we disagree with people we can treat them with respect and assume they're intelligent people acting in good faith. Unfortunately this blog post doesn't set a very good example in that regard.
Forgejo negatively in the news, just after several large GitHub outages and generally bad press for GitHub? Coincidence? </tinfoil_hat>