SiteSetting numbers not properly validated

AI-generated summary

SiteSetting numbers not properly validated

A bug was reported where entering a large value for certain site settings in the admin dashboard causes a 500 error upon reload. rcfox demonstrated this by setting embed_post_limit to an extremely large number.

codinghoror suggested adding default integer validation for site settings. However, cpradio pointed out that even entering reasonable values could potentially cause issues.

Discussion around the maximum possible integer value ensued, with Mittineague and codinghorror debating the limits of Postgre Int and Ruby’s handling of large integers.

elberet identified the exact line of code causing the issue and codinghorror assigned the task to techAPJ, who has since added number validation in a pull request.

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개의 좋아요

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:

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개의 좋아요

@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개의 좋아요