-1

I have a piece of code, in which I set true or false depending upon the conditions.

Below is that code

public bool HackerTextExistOrNot(string text)
    {
        bool flgValid = false;
        var attackChars = new char[] { '=', '+', '-', '@' };

        if(attackChars.Contains(text[0]))
        {
            flgValid = false;
        }
        else
        {
            flgValid = true;
        }
        return flgValid;
    }

I have checked for both the bool conditions, but it always goes in strReturnId in main function.

Below is the code.

public static string SaveRecord(RRSOCSaving RRSOCSaving, string Indication)
        {
            string strReturnId = "";
            string strAppURL = ConfigurationManager.AppSettings["AppUrl"].ToString();
            string strmail_Content = "";

            CommonDB commonObj = new CommonDB();

            GET_DATA_BY_STORE objGetData = new GET_DATA_BY_STORE();

            try
            {
                if (objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_CODE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STATE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.CITY) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SITE_STORE_FORMAT) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_SITENAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_SITENAME_LANDL_1) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_SITENAME_LANDL_2) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_ASST_MANAGER_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_ASST_MANAGER_MOBNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_MANAGER_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.MANAGER_MOBNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.EMP_NEAREST_STORE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.EMP_NEAREST_STORE_MOBNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SUPERVISOR_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SUPERVISOR_MOBNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SECURITY_SUP_NAME_STORE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SECURITY_SUP_MOBNO_STORE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NAME_ALIGNED_LPO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.LPO_MOBILENO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALPM_ALPO_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALPM_ALPO_MOBNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.AREA_MANAGER_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.AREA_MANAGER_MOBNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ZONAL_HEAD_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ZONAL_HEAD_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.DVR_IP_ADDRESS) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SIGNET_IP_ADDRESS) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NEAREST_POLICE_STN_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NEAREST_POLICE_STN_CONTNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NEAREST_HOSP_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NEAREST_HOSP_CONTNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NEAREST_FIRE_STN_CONTNAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NEAREST_FIRE_STN_CONTNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_ADDRESS) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_SPACE_SQFT) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.LAUNCH_DATE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.CST_TIN_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_EMAILID) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NO_OF_POS) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NO_OF_CAMERA) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.DVR_MODEL_GESECURITY) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.CAMERA_MODEL) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALIGNED_LPO_MAILDID) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.FACILTY_TEAMNAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.FACILITY_TEAMNO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STATE_HEAD_OPS_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.STATE_HEAD_OPS_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.LPA) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SLP_STATE_HEAD) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SLP_STATE_HEAD_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.UserName) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.CREATED_DATE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.UserName) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.LAST_UPDATED_DATE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ISACTIVE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.LATITUDE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.LONGITUDE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SLP_EMAILID) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ZONAL_ECNUMBER) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ZONAL_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SLP_STATE_ECNUMBER) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALPM_ALPO_ECNUMBER) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.IS_STORE_IN_MALL) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.MALL_CONTROL_ROOM_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.IS_NIGHT_SEC_GUARD_AVAIL) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NIGHT_SEC_GUARD_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.NIGHT_SEC_GUARD_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.IS_NIGHT_PATROL_PARTY_AVAIL) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.PATROL_PARTY_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.PATROL_PARTY_NO) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALPM_ALPO_EMAILID) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALIGNED_LPO_ECNUMBER) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.SLP_STATE) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.FORMAT_GROUP) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALPM_NAME) ||
                    objGetData.HackerTextExistOrNot(RRSOCSaving.ALPM_ECNUMBER))
                {
                    strReturnId = "Something went wrong due to malicious script attack..!!!";
                }
                else
                {

                    if (RRSOCSaving.ROLE_ASSIGNED == "SLP State Head")
                    {
                        bool blnState1 = Array.Exists(RRSOCSaving.ASSIGNED_STATE.ToString().ToUpper().Split(','), element => element == (RRSOCSaving.STATE).ToString().ToUpper());

                        if (blnState1)
                        {
                            strmail_Content = Get_Email_Content(RRSOCSaving.STORE_CODE, RRSOCSaving.UserName, Indication, RRSOCSaving.STATE, RRSOCSaving.SITE_STORE_FORMAT, RRSOCSaving.STORE_SITENAME);
                            //  SendEmail(RRSOCSaving.UserName, RRSOCSaving.STORE_CODE, RRSOCSaving.SLP_EMAILID, ConfigurationManager.AppSettings["NHQEmail"].ToString(), strmail_Content, Indication);
                            strReturnId = CommonDB.INSERT_INTO_RRSOC_INFO(RRSOCSaving, Indication);
                        }
                        else
                        {
                            strReturnId = "User can add data for " + RRSOCSaving.ASSIGNED_STATE + " only";
                        }
                    }
                    else if (RRSOCSaving.ROLE_ASSIGNED == "NHQ Admin")
                    {
                        strmail_Content = Get_Email_Content(RRSOCSaving.STORE_CODE, RRSOCSaving.UserName, Indication, RRSOCSaving.STATE, RRSOCSaving.SITE_STORE_FORMAT, RRSOCSaving.STORE_SITENAME);
                        // SendEmail(RRSOCSaving.UserName, RRSOCSaving.STORE_CODE, RRSOCSaving.SLP_EMAILID, ConfigurationManager.AppSettings["NHQEmail"].ToString(), strmail_Content, Indication);
                        strReturnId = CommonDB.INSERT_INTO_RRSOC_INFO(RRSOCSaving, Indication);
                        //strReturnId = "Record Saved Succesfully";
                    }

                }

            }
            catch (Exception)
            {
                throw;
            }

            return strReturnId;

        }

UPDATE I mean to say always in

strReturnId = "Something went wrong due to malicious script attack..!!!";

Palle Due
  • 5,929
  • 4
  • 17
  • 32
Nad
  • 4,605
  • 11
  • 71
  • 160
  • This question is a little confusing. You need an [mcve] – TheGeneral Oct 19 '21 at 09:08
  • `if(attackChars.Contains(text[0]))` is this condition correct ? – jophab Oct 19 '21 at 09:08
  • @jophab: yes, its correct.! – Nad Oct 19 '21 at 09:09
  • Can u please share the value of `RRSOCSaving.STORE_CODE` ? Does any values really starts with the attackChars ? – jophab Oct 19 '21 at 09:11
  • Additionally, your code *isn't* a Javascript snippet, so please don't format it as if it were. Note that you still haven't provided a [mcve]. – Jon Skeet Oct 19 '21 at 09:11
  • @jophab: THe value is `HENTEST` – Nad Oct 19 '21 at 09:11
  • "it always goes in strReturnId" - what do you mean by that? Please edit your question to make it *much* clearer. – Jon Skeet Oct 19 '21 at 09:12
  • As an aside, your `if` statement and the local variable can all be replaced with `return attackChars.Contains(text[0]);` – Jon Skeet Oct 19 '21 at 09:13
  • @JonSkeet: I have updated the question. May be its much clearer now. It always goes in first condition of `strReturnId = "Something went wrong due to malicious script attack..!!!";` even if its true or false – Nad Oct 19 '21 at 09:13
  • 2
    This is definitely not the way to guard against injection attacks anyway. You should use `StringComparison.OrdinalIgnoreCase` instead of `ToUpper`. And `catch { throw;` is just useless. – Charlieface Oct 19 '21 at 09:13
  • Your `HackerTextExistOrNot()` returns `true` if the string does NOT begin with any of '=', '+', '-', '@'. So if the string doesn't contain any of those, the big `if` will enter its body. This seems to be the wrong way around. – Matthew Watson Oct 19 '21 at 09:15
  • @Charlieface: If any other way then please let me know.. Would try that way if its according to my requirement – Nad Oct 19 '21 at 09:15
  • @MatthewWatson: So how could I do that? any suggestion – Nad Oct 19 '21 at 09:16
  • I'm not entirely clear what you are doing with the text you need to check. Is it SQL, or is it HTML or what? To guard against injection, you either use proper parameterization, or you escape it. Checking for "bad" input is an exercise in futility – Charlieface Oct 19 '21 at 09:16
  • @JonSkeet: It's actually return !attackChars.Contains(text[0]); – Palle Due Oct 19 '21 at 09:19
  • @PalleDue: Whoops, yes, absolutely. – Jon Skeet Oct 19 '21 at 09:26
  • The question was last updated *before* my comment saying how it was unclear. It's still unclear, and it still doesn't have a [mcve]. – Jon Skeet Oct 19 '21 at 09:27
  • @JonSkeet: Ok, See, I just want user to stop inserting some invalid characters.. SO for that I am implementing the code which I posted. Now that same code works same for both true and false condition which is wrong. I want to check and correct that. – Nad Oct 19 '21 at 09:31
  • 1
    "Now that same code works same for both true and false condition which is wrong" - I don't know what you mean by that. I suspect I *would* understand if you would just post a [mcve]. Hint: a minimal example wouldn't contain an `if` condition that had 76 lines of code... – Jon Skeet Oct 19 '21 at 09:40

1 Answers1

2

It seems like your

HackerTextExistOrNot

method returns true when hacker text does NOT exist. Instead of using flgValid just return attackChars.Contains(text[0]) and it should be working correctly.

One more thing - you are creating table each time entering this method, you might consider refactoring this code.

Piotr Wojsa
  • 938
  • 8
  • 22
  • returning like `return attackChars.Contains(text[0]);` is going in `catch` block. giving error as `Object reference not set to an instance of an object.` – Nad Oct 19 '21 at 09:30
  • Piotr, help with the code. The current `return` is giving error. – Nad Oct 19 '21 at 09:40
  • try changing it to return !string.IsNullOrEmpty(text) && attackChars.Contains(text[0]); – Piotr Wojsa Oct 19 '21 at 09:42
  • now I returned like this `return (!string.IsNullOrEmpty(text) && attackChars.Contains(text[0]));` but still getting the same error – Nad Oct 19 '21 at 09:48
  • Are you sure this line throws an exception? If so, then the only possibility is that attackChars is null. If it's not, the issue is elsewhere. – Piotr Wojsa Oct 19 '21 at 09:51
  • no this line doesn't throws any error, after this it goes to `if (objGetData.HackerTextExistOrNot(RRSOCSaving.STORE_CODE) || objGetData.HackerTextExistOrNot(RRSOCSaving.STATE) || objGetData.HackerTextExistOrNot(RRSOCSaving.CITY) ||` where it throws and goes to catch block – Nad Oct 19 '21 at 09:53
  • well, actually this line was supposed to be part of HackerTextExistOrNot method. So if it fails on these conditions then either objGetData is null or RRSOCSaving is null. If the exception is thrown further then it's not really part of this question – Piotr Wojsa Oct 19 '21 at 09:56
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/238313/discussion-between-hud-and-piotr-wojsa). – Nad Oct 19 '21 at 09:57