Readit News logoReadit News
orblivion · 5 years ago
No surprise to me. At one point when they had an API, I could follow and see posts from "locked" accounts that I couldn't see from the website. I reported it and they said something like "oh yeah we're making a lot of changes right now" and of course never fixed it (until they decommed their API which effectively fixes it).

They're up against the world, and they're spreading themselves very thin, aiming to replace all of the "big tech" use cases, for better or for worse. They hate Silicon Valley but they "move fast and break things" more than anybody, arguably by necessity. I'll cut them some slack if their public platforms have some bugs, but they shouldn't do anything with sensitive user data.

ugh123 · 5 years ago
I think a lot of it also speaks to the pool of quality engineers who are willing to work for a company like Gab, Parler, etc. Companies like that appearing on your resume will no doubt cause friction with would-be employers down the line, for better or worse. I certainly wouldnt work there unless I was absolutely desperate and couldn't look more than a few years down the road.
user-the-name · 5 years ago
I'd like to think people would not work for far-right hosts of hatred, racism, sexism and often terrorisms for other reasons than just "it would look bad on my CV". I would hope people would have enough of a moral compass to be able to tell that these companies are, in fact, not morally good.
lawnchair_larry · 5 years ago
It doesn’t say anything like that. I guess you missed the part about him being a facebook employee. Go ahead, say facebook hires low quality engineers...
zelon88 · 5 years ago
> I'll cut them some slack if their public platforms have some bugs...

Your generosity naively assumes that given enough time they would resolve your concerns. That is never the case with these anti-social social media companies.

It's taboo to talk about, but the correlation between political conservatism and overwhelming amounts of technical debt leading to non-existent operational security is hard to overlook.

carmen_sandiego · 5 years ago
I’m not sure it’s taboo to talk about as much as it’s just a totally baseless claim.

Deleted Comment

yrgulation · 5 years ago
The cto should focus on strategic thinking and have one or two devs that lead daily work - and code review for such basic issues (or use a code analyser) to detect sql, xss, xsrf, session management, password based user data encryption, message encryption, and other trivialities. Not that i like gab, but i wonder how many rookie mistakes like this one are then blamed on ”foreign agents” and other media bollox.
gkoberger · 5 years ago
You're right in general, however I think you're over-estimating the size of Gab.

In this case, it's more of a "everyone gets a CxO title because there's fewer than 26 of us" type thing.

Dead Comment

claytongulick · 5 years ago
I'm of the general mindset that the CTO should be the most technical.

I think we see a lot of tech company failures when the CTO is just a manager type that doesn't understand the technology and doesn't have the ability to contribute to it.

Deleted Comment

0x4d464d48 · 5 years ago
"move fast and break things"

More like "move fast and break yourself."

Dead Comment

abvdasker · 5 years ago
This is bad code and it's a little surprising to me that a former Facebook engineer wrote it (and subsequently became the CTO).

It's been a while since I wrote any Rails but the offenses that jump out just from a cursory inspection:

- large raw SQL query which could almost certainly be accomplished in a more idiomatic way with AREL or ActiveRecord

- no user input sanitizing

- using a regular string literal for a large text block instead of a Ruby here document

- leaving a mess of commented-out code at the bottom of the method

Apparently Gab isn't exactly hiring the best and brightest.

smoldesu · 5 years ago
I'm not terribly surprised on their hiring practices. I got a Gab account a few years ago for kicks, and I was banned within a week. I never got any justification for my ban, and after scouring my locked account for any offending posts I can only conclude that their moderators are horribly corrupt.

I love decentralized software, but don't let Gab fool you. They're just out to grab power right now.

np_tedious · 5 years ago
Idk when this guy worked there, but today a Facebook product dev likely doesn't get a chance to make a mistake like this. Which is good. But it also means people who've only worked there can have some really surprising gaps
shadowgovt · 5 years ago
It's always worth remembering that there's a difference between employment and a university degree. Universities don't grant degrees until someone has proved they have whatever competency the degree indicates. Employment means you got paid by the company, but what you did to get paid could be lots of things... And it's pretty easy to lie on a résumé. Universities have a reputation to lose if they grant a degree to someone who goes on to mess up badly; corporations have no such ties to a former employee, and Mr. Marotto making a major error when he's no longer employed by Facebook doesn't reflect poorly on Facebook.

"Former Facebook engineer" means Facebook paid him... No more and no less.

bashwizard · 5 years ago
Not to be that guy but some of the worst security vulnerabilities I've seen has been from senior devs with college degrees. Having a degree =/= you know your shit or being "immune" to absurdly stupid mistakes like this.

You'd be surprised how often I've found leaked API tokens, passwords in plaintext etc on github for instance.

mrunkel · 5 years ago
> Universities don't grant degrees until someone has proved they have whatever competency the degree indicates.

Universities grant degrees based on the successful completion of course requirements. I don't know if it's fair to conflate that with what you said. As long as you can get C or better in every course required, you have a degree. Does that necessarily mean that you are competent?

It's an even further stretch if you think that at University cares all that much if one of their Computer Science graduates makes a mistake.

kodah · 5 years ago
Software Engineer and former SRE-SE here; also a two year drop out.

> Universities don't grant degrees until someone has proved they have whatever competency the degree indicates

That is not a rule. I have interviewed countless folks with CS and CE degrees that can't get past basic scalability questions in abstract systems design. I've started plenty of hour long interviews only to learn five minutes in that the candidate has a four year degree and cannot correctly identify a use case for binary search nor implement it in plain code.

> Universities have a reputation to lose if they grant a degree to someone who goes on to mess up badly

They absolutely do not lose reputation. We don't publish on a website where the degrees came from that allowed Cambridge Analytica to exploit the American people. Everyone will easily write off the offending programmers lineage as a fluke and move on with their lives. Nobody really cares where you came from in this world, just what you're up to now.

> Mr. Marotto making a major error when he's no longer employed by Facebook doesn't reflect poorly on Facebook.

I agree that this would be a pretty trash take and a poor precedent to establish long term.

> "Former Facebook engineer" means Facebook paid him... No more and no less.

Simultaneously, this is not true. Working for Facebook carries some prestige, as we can see from all the folks who proudly sport "x-Facebook", "x-Google", "x-Netflix" show us. Obviously some people make use of this market and obviously someone is foolish enough to believe the sheen. The value in that perception rests with the viewer; if they hired him because of his work at Facebook then it was significant, if they would've hired him regardless then it matters very little.

tester34 · 5 years ago
>Universities don't grant degrees until someone has proved they have whatever competency the degree indicates.

haha

you'd be really shocked if you knew about the great lengths people go to get their degree and learning stuff ain't one of them :-)

ufmace · 5 years ago
Have you ever worked on any really large platforms or databases? IME it's pretty common that the idiomatic Arel/ActiveRecord code you might normally write can become hopelessly slow when the query gets complex and the datasets get large. I'm not trying to pick on ActiveRecord here, pretty much all ORMs I've ever used will do this sometimes. The only way to get the speed up to something acceptable may be for someone who knows SQL and the DB Schema well to write a custom SQL query that makes the most of the database indexes and optimizations.

When a large product in Production is having performance issues, it's very possible that you'll write something like that in a time crunch to get the site back up. The code might not be the cleanest possible because you aren't sure how well it will work yet. It might be checked into Git because any remotely sane deployment procedure requires it. And maybe somebody years later will pore through the Git logs and come back and drop snark on you without knowing anything about what was involved in writing and releasing that code and they've never had to deal with something like that.

AmericanChopper · 5 years ago
At a glance my first though was "there's probably SQLi here", but it's not obvious to me that attacker controlled parameters could actually be used to exploit this. The ID and pagination parameters all seem to be ints, and it's not obvious to me that an attacker could turn them into arbitrary strings. Is custom_filters.phrase the attacker controlled string?
kemayo · 5 years ago
There's nothing in that code that says that any of those parameters have to be ints. They're certainly expecting ints, but there's no casts in that code that would enforce that. Presumably somewhere upstream they're just passing directly from the POST/querystring into these API functions.

Remember: Ruby is a dynamically typed language.

robinduckett · 5 years ago
Why is it surprising to you that Facebook could hire someone inept?
abvdasker · 5 years ago
I mean I did say it's only a little surprising.
stickfigure · 5 years ago
> Developers: Sanitize user input

No, don't sanitize user input. Don't trust it, which is a big difference. Use bound parameters and this problem goes away.

acdha · 5 years ago
A better answer is both not trusting it and validating it: parameters prevent data from being interpreted as code but validation is also important because allowing unexpected inputs could lead to other vulnerabilities (e.g. XSS in an error message or a partially-generated page) or complications when making changes in the future if you learn that common clients have come to rely on your sanitization logic cleaning up their requests.
bcrosby95 · 5 years ago
Please no. Trying to validate your input for any format that currently exists or may exist in the future is really wrong. You need to handle this at the edges - for SQL this is using prepared statements, for XSS its about using a framework that makes the correct choice (default escape HTML entities).

Anything else is madness. Of course, if you're dealing with legacy systems you might have to do something tragic like not allow people to put quotes in passwords.

Deleted Comment

nuclear_eclipse · 5 years ago
Sanitize, normalize, and distrust. All three cover different use cases.
NotPavlovsDog · 5 years ago
This leaves a bad taste as to what the "forensics" could uncover just due to the open development model. It's nice to be the prophet of evident mistakes when the trail is easy to follow, even if you can't exactly master the lingo.

From the article:

   >The change, which in the parlance of software development is known as a “git commit,”
It was a change. Parlance commit. Parlance, per tool used, "git commit" (and then check as to tool standard parlance). My point being, what do we routinely hide thanks to not coding in public? What do engineers routinely hide, when possible?

I would rather, as an engineer, discuss core issues we can fundamentally address: compromising on inadequate workflows (including core architecture and paradigms), commitment to over-delivery, and the ever-dooming deadlines. What victories of the CTO went ignored, as part of "the job"?

It's nice to be smart when you have regular 8 hours of sleep. I've had enough stress to remember just how idiotic many of my decisions were, as a "leader". Most of them went ignored just because we were covered by being invisible, by design. Morally, I can't judge this CTO. If you look at your coding history, can you?

minimaxir · 5 years ago
There's a difference between commiting hacky-but-working code during an Agile sprint and commiting code that allows unsanitized input to a SQL query.
NotPavlovsDog · 5 years ago
Yes, but then, really, the fundamental popular software paradigm is not just unsanitized but unsanitary. The models for sanitary-by-design are there. It's just math.

   The core leadership behind inadequate decisions, often above the CTO, are frequently of the "don't care about the math, just the numbers" type.
Perhaps the CTO raised concerns. Maybe, not. But if we want an open engineering culture in software, unlike "applied engineering" in other industries, we should actively oppose punishing those that embrace open-to-peer-review models, even when the openness backfires and the history gets removed by the open workflow participants.

We may still have a fragile and unique culture in software, that perhaps contradicts past history such as engineering in construction (look up "corruption construction") or the unique corruption of medicine ("sugar lobby", "food pyramid").

Despite bad decisions and the fumbled cover-up, the attempt to perform in public on their part is valuable to me. We don't have easy access to which of the doctors took money to publish "research" that "calories are the same", pushing for more carbohydrates in diets. This may translate to multiple people, people you might personally know, dying of diabetes.

With open software, we get the names. This should not reward click-bait media witch-hunting.

bosswipe · 5 years ago
I can 100% judge him. It is the CTO's job to put in place processes and safeguards that reduce the possibility of one of the most common widely known security vulnerabilities. Either he didn't put in the safeguards or he bypassed them, either way it's a fireable offense that put the whole business in danger.
NotPavlovsDog · 5 years ago
Do you have your commit history available in a public repository? I don't. Honestly, i'm paid for being a professional fuck-up. I just fix things quickly and support my team enough for us to bear the mutual guilt in silence.
cwhiz · 5 years ago
The CTO has only been there since November. No idea what type of situation he or she may have inherited.

However, it looks like the CTO pushed this directly, no PR.

an_opabinia · 5 years ago
> commitment to over-delivery, and the ever-dooming deadlines.

Surely the difficulty in recruiting people to work for their shitty website with shitty politics should illuminate for you and everyone also in denial that politics is also engineering.

Lukas_Skywalker · 5 years ago
> „ Specifically, line 23 strips the code of “reject” and “filter,” which are API functions that implement a programming idiom that protects against SQL injection attacks.“

This is not correct. The mistake was to use ‚find_by_sql‘ without parametrizing the query. The mentioned reject and filter methods are merely skipping some of the data the query returns.

Delk · 5 years ago
The previous code that used those functions probably did prevent a SQL injection as a side effect, as using them avoided making a direct SQL query at all.

But you're of course correct that it's not the replacement of an ORM call with SQL that's the problem.

tomc1985 · 5 years ago
> “Sadly Rails documentation doesn't warn you about this pitfall, [...]" said Dmitry Borodaenko, a former production engineer at Facebook who brought the commit to my attention wrote in an email.

This is completely and utterly untrue.

https://guides.rubyonrails.org/security.html#sql-injection shows examples that are exactly like the code in question in that commit

Bound parameters were a new thing like 15 years ago.

kstrauser · 5 years ago
Maybe they were new 15 years ago in Rails, but I was using them in Perl in the mid-90s.

There is no excuse for writing an SQL injection in 2021. Zero. None. And if you're in the position to write code that'll be deployed to production, you darn well better have it reviewed by peers before it's merged.

If the CTO did this and worked around the developers, he's a freaking idiot. If the engineers saw this and signed off because he's the CTO, they're freaking idiots. I wouldn't ordinarily be this harsh about it, but come on. SQL injections in 2021? That's astoundingly bad.

rsynnott · 5 years ago
> Maybe they were new 15 years ago in Rails

That's when Rails came out :)

I'm fairly sure ActiveRecord has always supported them.

ratsmack · 5 years ago
I would think a Ruby code fuzzer would have caught something like that... if they have used one.
dragonwriter · 5 years ago
> Maybe they were new 15 years ago in Rails,

Well, Rails was < 2 years old then, so everything was new in Rails.

btilly · 5 years ago
Bound parameters were a new thing like 15 years ago.

When they were new depends on what language and database library you use.

Perl's DBI had them 25 years ago.

banana_giraffe · 5 years ago
And Visual Basic itself had them 23 years ago as well.

Though, the inflection point, at least in my circles, was in 1998, when rain.forest.puppy talked about "piggy backing" SQL commands.

Even there they mentioned using parameters with stored procedures to protect yourself. It's just clear no one in IIS land did at the time, so there was a wide open exploit playground for everyone that saw that release of Phrack, once they got done playing with directory traversal exploits.

AdamN · 5 years ago
Dear lord I'm getting old ... :-)
tomc1985 · 5 years ago
Probably; I vaguely remember them being discussed at my very first programming job around mid 2000's, which was writing Perl
GongOfFour · 5 years ago
The other funny thing is this bit:

> Specifically, line 23 strips the code of “reject” and “filter,” which are API functions that implement a programming idiom that protects against SQL injection attacks.

That's not true at all. If you are going to do technical analysis that calls out mistakes, the publication should have multiple technical people review the article.

theptip · 5 years ago
Unfamiliar with Ruby/Rails, why's that untrue? Is it because `reject` / `filter` are just array mapping functions rather than ActiveRecord methods? (Best explanation I could come up with on a quick skim.)
lilyball · 5 years ago
I had the same reaction, but upon reflection, the reject/filter actions themselves aren’t what’s safe, it’s the usage of functions that accept unsanitized input and safely wrap the SQL that’s going on here.
xref · 5 years ago
Come on man, don’t just comment “the article is wrong” then walk away. Put in some effort.
avgDev · 5 years ago
Yikes. I'm working on an internal web app which can only be accessed on our network and even I am using parametrized queries to prevent injection.

What a joke, and then blaming the documentation...just wow.

amatecha · 5 years ago
It looks like the Gab devs followed the documentation precisely then! Just... apparently they copied the wrong part?! heheh ;)
FanaHOVA · 5 years ago
It's like Twitter calling Rails LEGO because they weren't able to scale it :)
naebother · 5 years ago
> Besides the commit raising questions about Gab’s process for developing secure code, the social media site is also facing criticism for removing the commits from its website. Critics say the move violates terms of the Affero General Public License, which governs Gab’s reuse of Mastodon, an open source software package for hosting social networking platforms.

Lmao, that's not how AGPL works. Although, it would be pretty funny if a license explicitly required you to post all your Ls for public ridicule.

dragonwriter · 5 years ago
> Lmao, that's not how AGPL works.

The GPL family, including the AGPL, defines source code that must be provided as “the preferred form for making changes”; if the form the distributor uses is a VCS repository with full history, publishing either static snapshots or a repository with an expurgated history could arguably be a violation of both the letter and the spirit of the licenses. The language of the licenses makes the requirement dependent on actual development practices, not fixed, and that's not an accident.

naebother · 5 years ago
I'm guessing you're referring to the first sentence in section 1:

> The "source code" for a work means the preferred form of the work for making modifications to it. "Object code" means any non-source form of a work.

I don't think that's referring to the method by which I make changes or manage those changes. It's simply trying to make it clear that source-code is something that should be in a form you can actually change, and not something obfuscated. e.g. minified, transpiled, object-code, etc. The second sentence gives the first more context.

The spirit of all GNU licenses is that the end-user should have the freedom and ability to understand or modify software that's *distributed* to them for themselves. It has nothing to do with having a full history of the source-code available. What does full history even mean? I'd like every auto-save to every LGPL licensed file, please.

forgotmypw17 · 5 years ago
I've been pretty sure for a while that Parler and Gab were both honeypots. This makes it seem even more likely.
McGlockenshire · 5 years ago
Never attribute to malice that which can equally be explained by incompetence, or something like that.
adolph · 5 years ago
Never assume xor among different interpretations.

Dead Comment

GVIrish · 5 years ago
Or maybe they just didn't build a strong enough engineering team and quickly ran into the limits of their experience and skills levels.

There are numerous examples of start ups and even mature companies making basic mistakes. This is easily explainable without resorting to conspiracy theories.

eternalban · 5 years ago
> maybe they just didn't build a strong enough engineering team and quickly ran into the limits of their experience and skills levels.

"Free Speech platform Gab has announced Fosco Marotto as the company’s new Chief Technical Officer (CTO). According to a blog post from the company, Marotto was formerly a software engineer, production engineer, and developer advocate during a seven-year career at Facebook.

Marotto reportedly brings 23 years of industry experience to the platform along with extensive knowledge in backend infrastructure and insight that will help Gab scale as it becomes increasingly popular."

CTO: https://github.com/gfosco

Parse Server: https://github.com/parse-community/parse-server

Parse Server API doc snippet:

"If your app is compromised, it’s not only you as the developer who suffers, but potentially the users of your app as well. Continue reading for our suggestions for sensible defaults and precautions to take before releasing your app into the wild."

https://docs.parseplatform.org/rest/guide/#security

Per Ars, he removed security code.

btmcnellis · 5 years ago
"solarwinds123" comes to mind.
JohnTHaller · 5 years ago
Malice or incompetence? Why choose? They've got both!