SiteSetting numbers not properly validated

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.

1 Like

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:

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…

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

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…

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

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.

Actually, it’s not that bad.

It breaks here on line 362:

https://github.com/discourse/discourse/blob/master/lib/site_setting_extension.rb#L347-L364

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

1 Like

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

Okay, added number validation:

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

3 Likes