Is this code vulnerable to SQL Injection? How would I make it safe?

Question

I have a friend who I'm trying to help out on a project, I'm by no means an expert at all in penetrating testing, in fact it's something I'm really new in, but he's asked me to help identify and potential security issues with his program.

I'm not too sure but I have a feeling that in particular one segment of code is vulnerable to SQL injection, I was wondering if anyone could provide examples of how it may be vulnerable (if it is) and also any suggestions about how the input should be sanitised.

Here is a snippet of the code

nextwork:
        cDataBase *db = NewManagerDB();
        if( !db->QueryDirect( dbginfo,  "SELECT * FROM Account WHERE UserID = '%s'", szManagerID ) )
        {
                ErrDB(db);
                DelManagerDB(db);
                if( Retire == MAX_RETIRE )
                {
                        LOG("QueryDirect error MAX_RETIRE db QueryDirect failed");
                        SendManagerAuthenResult( MgrCode::SvrError );                        
                        return;
                }
                Retire++;
                goto nextwork;
        }

Thank you for any help!

Are you munging query strings with user input? Yes. So your code is vulnerable. Do you use parameters to pass in constant values? No. So you need to learn how to do that. — Gordon Linoff, Apr 12 '19 at 12:25
Possible duplicate of [How does the SQL injection from the "Bobby Tables" XKCD comic work?](https://stackoverflow.com/questions/332365/how-does-the-sql-injection-from-the-bobby-tables-xkcd-comic-work) — Marek R, Apr 12 '19 at 12:52
@WGHaven, I would appreciate you reviewing my answer to your question and if I was able to help you, marking answer as accepted by clicking on `v` under the answer's score. Thank you — Sergey Nudnov, Apr 14 '19 at 19:38

score 1 · Answer 1 · answered Apr 13 '19 at 04:26

How to sanitize (not sure 100% how it will be with your particular db class implementation):

        db->Parameters.AddWithValue("@userid", szManagerID);  
        if( !db->QueryDirect( dbginfo,  "SELECT * FROM Account WHERE UserID = @userid" ) )

Here is an example of SQL injection:

szManagerID = "' OR UserID LIKE '%";

The result query will be:

SELECT * FROM Account WHERE UserID = '' OR UserID LIKE '%'

So you could get account data of all users

Is this code vulnerable to SQL Injection? How would I make it safe?

1 Answers1