-3

I wrote some code for mysql, at first i had it all parameterized. Then later a someone told me that it wasn't safe any-more. Its an old program i try to fix, but the standard input queries where not safe against injections. Though there are a lot of 'other' mysql things happening in the code, there are not much areas where new data is generated, or open user queries. So then i thought (not to endup in a battle of whats the best injection proof method), lets reformat the input strings so we never can get in such situations. I wrote a regex to always have properly formated varchar input fields

I am now using this for that:

public string AllowedAsci(string input, string symbol="*")
{
   return Regex.Replace(input, @"[^a-zA-Z0-9-+_@., ]", symbol);
}

That's basically a strict regex for basic Email and numbers, my question is can this regex be extended width other safe to use symbols.

Important update

  • The point of this question never was to raise a discussion about using mysql parameters, i knew that from the beginning, though politics are at work, here are branches of code who am i not allowed to touch. For the moment i have no intentions to get (again) into an argue at work nor to touch that code, i'l blame them in the end maybe but its political.

  • So please stay on topic what is a good regex to remove escape codes but on the other hand allow strings that dont allow for injection.

  • the regex rule does protect against all injections that i know of unless you can can proof me wrong width a better regex.
23
  • 9
    “Then later a someone told me that it wasn't safe any-more.” Someone was wrong or this is incomplete information Commented Apr 11, 2018 at 6:36
  • 1
    Could it be that there was some kind of miscommunication between you and whoever told you that parametrized queries are "not safe anymore"? Can you include an example of your old code? Commented Apr 11, 2018 at 6:39
  • 1
    I'm voting to close this question as off-topic because it is based on a false premise that "parameterized code is not safe". Commented Apr 11, 2018 at 6:42
  • 1
    Also, if this code is supposed to allow email addresses you need to include a lot more characters which would make your Injection Proofing kind of useless. Commented Apr 11, 2018 at 6:42
  • 1
    @user3800527 "I dont wat to get into a conflict again, there should be some better regex" No, that's not the right way to go. You are risking security to avoid conflict and when something goes wrong you will be at fault. Commented Apr 11, 2018 at 7:37

1 Answer 1

0

I guess we could answer that it doesn't matter if your function is safe, because your coworker's premise isn't safe to begin with.

One problem is that even if you allow only "safe" characters, there are ways of encoding malicious input using the safe characters, yet it does something unsafe.

I found this example of a malicious input in this old answer: https://stackoverflow.com/a/45099/20860:

DECLARE%20@S%20VARCHAR(4000);SET%20@S=CAST(0x4445434C415 245204054205641524348415228323535292C40432056415243

(I've only shown part of the malicious payload. The point is it's possible to exploit SQL injection through a filtered set of characters you would assume is safe.)

I'll also point out that if your expression is meant to allow email addresses, it won't. There are many other characters that are legal parts of email addresses, including some like " that would result in SQL injection vulnerabilities (even without the encoding trick I mentioned above).

See a long list of test data for email address patterns here: https://fightingforalostcause.net/content/misc/2006/compare-email-regex.php

If you work for someone who would fight you instead of thank you for identifying a security flaw, you should talk to your mutual manager privately and ask to be transferred to another department. If that can't be done, start looking for a different job.

In the meantime, do what they tell you to do, but insist on a code review. Get them to approve it, and write an email to your manager with evidence that your code was approved even with the security flaws.

Sign up to request clarification or add additional context in comments.

2 Comments

That was a reason for me to not include the % and the ; also not included & and ( ) ; your right about the other job, but finding one here is a bit complex, i could get a job elsewhere (big city) but my personal life binds me to a traffic jam free 20km range (my GF is often ill supporting here is my first goal).
You might just have to swallow your pride, code this function they way they want you to, and forget about it. But keep some evidence that it was their decision, for the court case. :-)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.