Is 'cast' dangerous in user-submitted content in classic ASP?

Question

The legacy web app I have inherited, which was custom-written for Oxfam New Zealand in classic ASP, runs a string replace on user-submitted inputs removing the string 'cast' presumably because of the cast function.

However this means that none of our participants can have a name or email address that contains that string. This is causing problems for someone with the surname Hardcastle.

This seems completely over the top security-wise - or at least there must be a way to ensure the user inputs are safe without changing the inputs of people with 'cast' in their name or email address.

The actual replace is done with:

strString = (Replace(strString, "cast", "", 1, -1, vbTextCompare))

I'm considering just commenting that line out, would that be safe to do?

score 2 · Answer 1 · answered Dec 14 '10 at 01:24

2

The legacy app is doing it wrong.

Rather than filtering the content at the source, the content should be property encoded wherever it is used. In other words, if it's being used in a query, the value would be encoded prior to adding it to the SQL statement or better yet placed unmolested into a stored procedure parameter.

So yes, you can remove that code, but make sure strString is being used safely elsewhere.

answered Dec 14 '10 at 01:24

Jacob

77,566
24
149
228

Thanks for your input, I may have been misleading about exactly when the string replace is done, I haven't actually looked to see when it happens, all I know is that there is a Function clean(strString) and that line is one of several lines with different strings being replaced. It may indeed be used just before it goes into the SQL and not after it is received from the user submission. – Stuart Young Dec 14 '10 at 05:11
In fact, the content in question is stored in the database including the 'cast' and so it seems certain it isn't done after submission, but before being used. – Stuart Young Dec 14 '10 at 05:17

score 0 · Accepted Answer · answered Dec 14 '10 at 01:08

0

I could change it to

strString = (Replace(strString, "cast(", "", 1, -1, vbTextCompare))

that way, you still get the "safety" of escaping the SQL, but won't aggravate users with cast in their names

answered Dec 14 '10 at 01:08

Sparky

14,967
2
31
45

Seems a reasonable quick fix and alternative to me commenting it out completely. – Stuart Young Dec 14 '10 at 05:12
Sure, but Jacob solution (stored procedure) is a better long-term security solution. The above is a only a quick fix... – Sparky Dec 14 '10 at 11:16

score 0 · Answer 3 · edited May 23 '17 at 10:33

This is actually an extremely ineffective way to prevent SQL injection attacks. There are 100 other words you would need to replace. The accepted answer is incorrect in that this does not retain the safety of this code, because there is no safety to begin with. The line with the "(" character would never execute if the hacker just added a space between CAST and (.

See this question on parameterized queries in classic asp. You never want to concatenate user supplied data to make a sql string. There are ways to do this without concatenation that are correct, but the code you currently have is useless. You might as well just remove that line.

Is 'cast' dangerous in user-submitted content in classic ASP?

3 Answers3