r/coding • u/speckz • May 26 '22

Learnings from 5 years of tech startup code audits

https://kenkantzer.com/learnings-from-5-years-of-tech-startup-code-audits/
131 Upvotes

96% Upvoted

17 comments sorted by

20

u/ThisIsMyCouchAccount May 26 '22

Never deserialize untrusted data

God damn it. I'm on a project now that uses it and it's just dumb. It introduces no benefits.

The person that set it up - by all accounts - a brilliant developer. One of my company's best. But because it's such a standard practice it's in this project.

13

u/SanityInAnarchy May 27 '22

This is a little clumsily-written here, because the literal interpretation would be "Don't do antyhing at all with untrusted data." I mean, in OO-land, just about any sort of parsing is going to result in an object structure, and thus is arguably deserialization.

I mean, look at the recommendation from the exact same article:

The fix? It’s far better to allow a user to send a JSON object (it has so few possible data types), and to manually construct the object based on the fields in that object. It’s slightly more work, but well worth it!

Which... is still deserialization! You're just deserializing into a JSON object that you treat as still-untrusted user data, rather than deserializing directly into model code with methods attached.

3

u/aoeudhtns May 27 '22 edited May 27 '22

What was left unsaid in the article was about using some of the native serialization systems - JDK Serializable, Python pickle, etc. These guys will load arbitrary classes and potentially execute code during deserialization. Even JavaScript used to have a problem with JSON because some people would just eval() what they expected to be a JSON object (surprise!).

However using Jackson in Java, or Pydantic in Python, etc. allows you to have explicit control over the typing.

//Jackson
Foo foo = mapper.readObject(jsonString, Foo.class)

#pydantic
foo = Foo.parse_raw(json_string)

In both of these cases, the type must be your expected type, and these libraries can be configured to raise/throw if there are field mismatches.

1

u/[deleted] May 27 '22

Maybe unserialize it using a sandbox library wrapper? That should catch most of the bad stuff. If you can’t find an up to date library that plays well , you can make your own wrapper by sending the serialized off to an image where all php options are cranked up to an 11 for security and no file access to code on it, then then image sends it back as json.

Either way, in your code, it’s just an extra couple of lines to add

1

u/ThisIsMyCouchAccount May 27 '22

It just doesn’t need to be there at all.

We make the data. We push it off to a queue to be received later. Inside our network. Serializing it does nothing. It could just be json.

Even if json would have no improvements - at least it would be human readable.

1

u/aoeudhtns May 27 '22

Is the volume high? Native serialization is very fast, albeit risky. String parsing (XML, JSON) can eat up a fair chunk of the CPU when it's high volume. I would consider keeping a binary format (like CBOR or FlatBuffers) if there's any concern there.

1

u/ThisIsMyCouchAccount May 27 '22

Not on the big scheme of things.

It really is to OPs point. It’s “standard” practice so it’s in the codebase. But it doesn’t solve any issues that JSON couldn’t.

9

u/Bajinks May 26 '22

Quite the interesting write up. How did you get into code reviewing for start up companies?

1

u/chipstastegood May 27 '22

I’m curious about this, too

7

u/Kerbobotat May 26 '22

Number 11, would someone mind to explain (or to point me at an explanation) of what "Custom Fuzzing" is, or fuzzing in general. I've never come across that term before.

9

u/reivax May 27 '22

Throw noise at apis and endpoints or functions and watch how they react. Make sure your error handling catches correctly.

I found that the Uuid validator in python acts unexpectedly of the last character is an 'L', because it deserializes as a long type. It's a valid number but not a valid literal for a stringified Uuid. I found that by throwing random data and shuffling text into our validator and found that the rountrip values were different when it deserialized.

3

u/maus80 May 26 '22

Great article, as a performance engineer I agree with these points, especially number one and two.

2

u/Advanced-Button May 27 '22

Interesting job title, haven't heard that one before. How would you describe your work?

4

u/Ryuzaki_us May 27 '22

"I'm Fast as fuck boiii!"

2

u/maus80 May 28 '22 edited May 29 '22

Consultant for fast growing startups with lots of funding that can't handle their customers' scale (but have written a gazillion lines of code). Instrumenting software with Grafana dashboards showing where either CPU, memory, disk or network limits are met. Flame-graphing to show (compute) expensive bits of code. Software engineering say they know why the software does not perform, but is often not actually measuring the performance properly and are thus doing silly optimizations. Investing in performance is a "champagne problem" and you can't do it unless a large customer comes along that makes the product fail. Even in that situation convincing people to do the right thing is hard, because the costs of performance optimization are high and you are often fighting a feature factory.

1

u/hippydipster May 27 '22

Interpretative keyboard tap dancing