Lets run Arbitrary SQL code!

PleegWat

@darkmatter Welcome to nested loop join land.

Dragnslcr

@anotherusername said in Lets run Arbitrary SQL code!:

I'm actually really curious if someone had a realistic idea of whether my transactions idea could work.

As far as I know, certain things will automatically commit the transaction, like dropping tables. Starting and rolling back a transaction around the query should help with some things, like accidental updates, but it definitely won't prevent any kind of dedicated attacker.

Tsaukpaetra

@darkmatter said in Lets run Arbitrary SQL code!:

coalesce(a.id,b.id) = coalesce(b.id,a.id)

anotherusername

@Dragnslcr well, let's hope the user doesn't have permissions to do that. Just because they need UPDATE/DELETE permissions doesn't mean they need DROP TABLE `users` permissions...

Tsaukpaetra

@anotherusername said in Lets run Arbitrary SQL code!:

Just because they need UPDATE/DELETE permissions doesn't mean they need DROP TABLE users permissions...

Yes. The user I had gotten created doesn't have DDL privileges anymore (the prior creds did, and that was an emergency situation that got me the current account, and it took six months). So, in theory you couldn't use simple DROP statements, but it should be rather easy to loop through all the databases available and DELETE from them... That should be transactionable though, maybe?

darkmatter

@Tsaukpaetra I mean.. it works fine provided there were no null keys
but then they take it a step farther to produce something similar to this thing that nearly killed our server:

select * 
from (select f.id _id,e.* from one_billion_row_table e left join smaller_sparse_table f on e.id=f.surrogate_id) a
join (select h.id _id,g.* from one_hundred_billion_row_table g left join smaller_sparse_table h on g.id=h.surrogate_id) b
on coalesce(a._id,b._id) = coalesce(b._id,a._id)

and VOILA!

_{didn't feel like typing a bunch of column names so I used *

and yes this result is next to meaningless without context as presented above... I could barely figure out what the real query they submitted was trying to do even WITH context.}

Tsaukpaetra

@darkmatter

dkf

@Tsaukpaetra said in Lets run Arbitrary SQL code!:

The user I had gotten created doesn't have DDL privileges anymore

DML privs are still quite enough to cause trouble, especially from a business perspective. Is there any particular reason to not stick with letting the arbitrary-access types just have DQL? That's possible for the DB engine to enforce easily enough, and it shouldn't be able to do worse than DoS things under normal circumstances.

DoctorJones

@Tsaukpaetra said in Lets run Arbitrary SQL code!:

unfortunately no, there isn't any current plans to add a new read-only user, as easy as it would be, due to bureaucracy

@boomzilla said in Lets run Arbitrary SQL code!:

I think this was really your only hope. What sorts of users are you planning to open this up to?

Couldn't you just set the transaction isolation level to read only?

It depends on what SQL engine you're using though.

Jaime

@darkmatter said in Lets run Arbitrary SQL code!:

but then they take it a step farther to produce something similar to this thing that nearly killed our server:

That's why the Resource Governor was invented. With a low-privilege account and the resource governor configured properly, you can give users ad hoc query capability with very few downsides. However, the user can still cause problems like this:

BEGIN TRANSACTION
SELECT * FROM important_table (TABLOCKX HOLDLOCK)

dkf

@Jaime said in Lets run Arbitrary SQL code!:

However, the user can still cause problems

That's why you need to record exactly who is issuing such problematic instructions, so you can go around with your friends Vinnie and Ron from the Concrete Co. and “persuade” them not to do that again.

Tsaukpaetra

@dkf said in Lets run Arbitrary SQL code!:

Is there any particular reason to not stick with letting the arbitrary-access types just have DQL?

Nope, just that it takes an arbitrarily long time to get a user created with just those privileges.

@DoctorJones said in Lets run Arbitrary SQL code!:

Couldn't you just set the transaction isolation level to read only?

I don't think that's how MSSQL transactions work... :/

@dkf said in Lets run Arbitrary SQL code!:

That's why you need to record exactly who is issuing such problematic instructions, so you can go around with your friends Vinnie and Ron from the Concrete Co. and “persuade” them not to do that again.

DoctorJones

@Tsaukpaetra said in Lets run Arbitrary SQL code!:

I don't think that's how MSSQL transactions work...

I didn't have time to read the thread too thoroughly, but didn't see any mention of which SQL engine you were using.

I don't think MSSQL supports a transaction isolation level of read only, it doesn't appear so from looking at the doc pages.

That's a shame, it'd have almost been too easy.

Lorne Kates

Option 1: Instead of EXEC, it runs a CLR. That CLR sends you an email with the statement about to be executed. If OK, you log into the server and run the statement.

Option 2: Just don't let statements with DELETE FROM or other "bad" keywords get through. Also, learn the wonders of EXEC CAST(44524F50204441544142415345 as nvarchar(max)) the painful way.

Option 3: Literally murder everyone who is denying you the ability to create a limited-rights user, eat their hearts, mount their severed heads in the IT room as a warning to everyone else, then with blood-stained fingers, create the limited-rights user.

Note: Any one of these stupid options is orders of magnitude better than any solution that doesn't use a limited-rights user.

Tsaukpaetra

@Lorne-Kates said in Lets run Arbitrary SQL code!:

the wonders of EXEC CAST(44524F50204441544142415345 as nvarchar(max)) the painful way.

Strange, Google only suggests this post as explanation for this. What wonder is this?

lolwhat

@Tsaukpaetra said in Lets run Arbitrary SQL code!:

What wonder is this?

Try running it, and let us know how it goes.

tufty

My first thought was to wrap stuff in a transaction. But then I thought of malicious_action; commit, regardless of any auto-committing that the database might carry out, and which has presumably been mentioned earlier anyway.

Really, the only option is to cut through the red tape stopping you from creating a read-only user. With a sharpened axe. Or even a blunt, rusty, one.

Arantor

@Tsaukpaetra he's taken DROP DATABASE and encoded it as hex as if it were a binary blob.

For every level of idiot-proofing there is always a better class of idiot waiting to be discovered.

Luhmann

@Arantor
Are you saying @Lorne-Kates is a better idiot?

Arantor

@Luhmann for some context ;)

Remember that we never ascribe to malice what can be explained with incompetence though Lorne went straight to the "explained with malice" example because he's Lorne and counter-culture to his native and went all "fuck you, give me drop database" rather than say "fuck you, give me inserted shitty data" like users might do.

Groaner

@Lorne-Kates said in Lets run Arbitrary SQL code!:

Option 1: Instead of EXEC, it runs a CLR. That CLR sends you an email with the statement about to be executed. If OK, you log into the server and run the statement.

Why go through all that trouble when there's a perfectly fine stored procedure that will do that for you?

Option 2: Just don't let statements with DELETE FROM or other "bad" keywords get through. Also, learn the wonders of EXEC CAST(44524F50204441544142415345 as nvarchar(max)) the painful way.

Msg 102, Level 15, State 1, Line 1
Incorrect syntax near 'F50204441544142415345'.

You'd have to prepend 0x for that to be interpreted as hex, and if you did, you'd get this:

剄偏䐠呁䉁十E

because you're casting binary straight to UCS-2.

Luhmann

@Arantor said in Lets run Arbitrary SQL code!:

"fuck you, give me drop database"

I guess we'll be waiting for his next spawn cycle to produce Bobby Tables Kates.

ben_lubar

@Lorne-Kates what about just pre-parsing the query and only accepting it if it's using a strict subset of safe SQL?

Groaner

@ben_lubar said in Lets run Arbitrary SQL code!:

pre-parsing the query and only accepting it if it's using a strict subset of safe SQL?

How do you propose doing that?

ben_lubar

@Groaner anything that isn't on a strict whitelist gets denied.

SELECT fieldName FROM tableName WHERE fieldName = constantValue

Scarlet_Manuka

@ben_lubar: he mentioned in the OP that words like "Drop" and "Update" are included in some of their column names, so you'd need some intelligence in the parser. Might still be doable particularly if you can exclude stuff like EXEC and CAST, but if those are allowable column names too it might get awkward.

dkf

@Scarlet_Manuka said in Lets run Arbitrary SQL code!:

you'd need some intelligence in the parser

Might be worth actually using a real parser and not just some random collection of hacked-together regexps. :)

Scarlet_Manuka

@dkf said in Lets run Arbitrary SQL code!:

a real parser

What sorcery is this?! Next you'll be saying he should use something that's already built and tested, rather than just bodging something together himself!

Tsaukpaetra

@Scarlet_Manuka said in Lets run Arbitrary SQL code!:

use something that's already built and tested

Like tossing the query at the server with a "don't actually execute this, but tell me what it does" flag? ;)

Apparently there are things you can put in your logic to disable execution, but if the attacker knows it, it should be just as easy to splice in SET FTMONLY OFF SET NOEXEC OFF SET PARSEONLY OFF and negate the idea entirely.

Lorne Kates

@ben_lubar said in Lets run Arbitrary SQL code!:

@Groaner anything that isn't on a strict whitelist gets denied.

SELECT fieldName FROM tableName WHERE fieldName = constantValue

If only there were some safe way to store this procedural operation...

loopback0

@Lorne-Kates If only read-only users existed...

ben_lubar

@loopback0 even with a read-only user, someone could write something with an infinite loop in it. At least with the strict parser-based validation, the only malicious thing that can happen is a terribly optimized query.

dkf

@ben_lubar said in Lets run Arbitrary SQL code!:

even with a read-only user, someone could write something with an infinite loop in it.

It's quite possible to write code that is clearly provably finite but takes an entirely unreasonable amount of time. Four nested loops, each over a collection of a million items… finite, but not going to terminate in the next millennia.

Jaime

@ben_lubar said in Lets run Arbitrary SQL code!:

someone could write something with an infinite loop in it

Microsoft makes long queries really easy.

WAITFOR DELAY '20:00:00'

However, the user would only be hurting themselves.

My earlier example would still defeat most of these ideas ...

BEGIN TRAN
SELECT * FROM important_table (TABLOCKX HOLDLOCK)

ben_lubar

@Jaime except that you probably wouldn't add table locking or waitfor to the whitelist.

Lorne Kates

Oh if only there was a Really good, known, stable Method for letting users work with database data without actually accessing the database proper.

IF ONLY!

Tsaukpaetra

Whelp, I finally got around to doing it. End code:

//Execute the script to check validity (Not really safe, but handles most blunders somewhat OK)
string testQuery = "SET ROWCOUNT  0 BEGIN TRANSACTION Select top 0 * into #T from(" + updated.Query + " )t ROLLBACK";
System.Data.Entity.Infrastructure.DbRawSqlQuery dtest = db.Database.SqlQuery(typeof(object), testQuery);
try
{
    await dtest.ToListAsync();
}
catch (Exception)
{
    throw new Exception("Query Broken, please review Refresh documentation for common problems and fixes.");
}

try
{

    updated.MODIFIED_BY = User.Identity.Name;
    updated.MODIFIED_ON = DateTime.Now;
    db.Entry(updated).State = System.Data.Entity.EntityState.Modified;
    db.SaveChanges();
    return Ok(updated);
}
catch(Exception)
{
    throw new Exception("Something bad happened.");
}

I can't wait to see who manages to break this!
Ideally, nobody will ever be smart enough to even think of trying, but if they do, their name and time is logged at least...

Tsaukpaetra

@Tsaukpaetra said in Lets run Arbitrary SQL code!:

doing it.

Splatting around in some other area, I decided to see what happens when I hack the front end to supply an invalid ID:

0_1473204312740_upload-04194b1e-cbdc-4716-a1d0-0c9e04385714

Hey, it works!

Edit: Underlying exception:

0_1473204379478_upload-78c82d70-037c-423e-885e-c893131198f7