SiteSetting numbers not properly validated


(Ryan Fox) #1

Entering a large value for some values in the admin settings can cause the settings dashboard to 500 upon reload. As an example, setting embed_post_limit to 999999999999999999999999999999999999999999999999999999999 will be successful, but breaks the settings page.

I was able to set it back to a non-broken value from the Rails repl, but I suspect most people won’t know how to do that.


(Jeff Atwood) #2

We should probably add some kind of default integer validation for site settings.

Still, it’s hard to read this as anything other than “when you do something dumb, something dumb happens” :wink:


(cpradio) #3

Depends. Sure he used an extreme example, but what if he entered 574 and it did it? Or 1000? Or 10000? At one point does it become “something dumb”?

I’m actually a bit surprised this hasn’t come up yet. Even with all of the playing around I’ve done with Discourse…


(Mittineague) #4

Assuming the value is a Postgre Int (signed 4 bytes) - negative values not used - the possible max value is

2,147,483,648 (if my maths serves me correctly)

Even if

999999999999999999999999999999999999999999999999999999999

was an overly extreme example it seems to me that accepting over 2 Billion is a more than generous


(Jeff Atwood) #5

Well, if you know programming at all, it’d likely be near 231.

Basically -2,147,483,648 to 2,147,483,647 so… pretty far outside the realm of normal.

Looks like Ruby does other magic here…


(cpradio) #6

Oh I do. And I know the the max storage of an int, but that didn’t mean it was stored that way… :smile:


(Mittineague) #7

That could explain the 500 errors. No telling what would happen. INSERT / UPDATE query FAIL, loss of data integrity, Clash of the Titans?

I guess one could argue that input validation should happen. Though it doesn’t seem it should be needed for Admins that should know better.


(Jens Maier) #8

Actually, it’s not that bad.

It breaks here on line 362:

It looks like writing and reading the invalid setting to/from PostgreSQL works nicely, but this method just can’t handle a Ruby Bignum


(Jeff Atwood) #9

@techapj this might be a good one for your list. Just fix number validation to start.


(Arpit Jalan) #10

Okay, added number validation:

https://github.com/discourse/discourse/pull/3393


(Arpit Jalan) #11