"Your Code Has a SQL Injection!" | Code Cop

Поделиться
HTML-код
  • Опубликовано: 1 ноя 2023
  • Use code GRPC20 and get 20% off the brand new "gRPC in .NET" course on Dometrain: dometrain.com/course/from-zer...
    Become a Patreon and get special perks: / nickchapsas
    Hello everybody, I'm Nick, and in this video, I'll show you what SQL Injection actually is and explain why people on LinkedIn shouldn't be talking about security and things they don't understand.
    Workshops: bit.ly/nickworkshops
    Don't forget to comment, like and subscribe :)
    Social Media:
    Follow me on GitHub: bit.ly/ChapsasGitHub
    Follow me on Twitter: bit.ly/ChapsasTwitter
    Connect on LinkedIn: bit.ly/ChapsasLinkedIn
    Keep coding merch: keepcoding.shop
    #csharp #dotnet

Комментарии • 254

  • @kaizer-777
    @kaizer-777 7 месяцев назад +225

    Getting programming advice from LinkedIn is like getting your news from Facebook.

    • @chpsilva
      @chpsilva 7 месяцев назад +4

      For real... It's bad enough when sometimes you ask in places like SO which supposedly would be a reliable source of knowledge and gets a bad advice; but if you don't even know where you can find good information... :facepalm:

    • @ErnaSolbergXXX
      @ErnaSolbergXXX 7 месяцев назад

      So where should we get news from today? I guess your not one of the few who still belive mainstream media is the source for news?

    • @chpsilva
      @chpsilva 7 месяцев назад

      @@ErnaSolbergXXX dunno, but I am talking about technical advice specifically. LinkedIn is not a good place to look for.

    • @kaizer-777
      @kaizer-777 7 месяцев назад +4

      @@ErnaSolbergXXX You should get your news from as many sources as possible so you can draw your own conclusions. Social media is no better than mainstream media unless you're just looking for validation in an echo chamber.

    • @keit99
      @keit99 7 месяцев назад +1

      ​@@kaizer-777also if you speak a foreign language, Look at sources in that language too. Don't use just one source, diversify then and draw your conclusions based on them.

  • @CryShana
    @CryShana 7 месяцев назад +133

    You can almost _feel_ the anger emanating from Nick. I can relate.

    • @klocugh12
      @klocugh12 7 месяцев назад +3

      I can def see why he would be when someone tries to get preachy about SQL injection without even knowing what it is. Like most here look at code and think "ummmm, how exactly do you do SQL injection with an integer parameter?"

  • @hermand
    @hermand 7 месяцев назад +149

    Well, I' only a few minutes in and I've just seen the code snippet and I look forward to seeing how a C# Integer can be used for an injection attack...... I love what you're doing with these videos, sick of seeing authoritatively presented nonsense by people who clearly don't have the first clue about what they're talking about. They're clearly just regurgitating things they've read without understanding

    • @billy65bob
      @billy65bob 7 месяцев назад +5

      In some other languages, just doing a 'ToString' of an number will include thousand separators and others even use COMMA as the decimal point.
      So the worst that will happen here is that the query blows up from a 'syntax error' or you get completely the wrong thing because you're from Europe and '100.000' was read as 100.
      As for .NET, I don't think anything like that would happen unless you're concatenating floats/doubles into queries this way.

    • @QvsTheWorld
      @QvsTheWorld 7 месяцев назад

      Same, I though I was going to learn something new.

  • @davideglass
    @davideglass 7 месяцев назад +11

    There is actually a way to get the int version to be injectable, though it is much harder and requires access to the machine running the code. If you can set the culture settings, for example: culture.NumberFormat.NegativeSign = "1' OR 1=1 --";
    Then pass in a negative number, that would output "SELECT * FROM Newsletter WHERE Id = '1' OR 1=1 --5'"

    • @juanprosales
      @juanprosales 7 месяцев назад +5

      Haha if are a hacker and have access to the machine running the code, why would you need SQL injection? 😂

    • @davideglass
      @davideglass 7 месяцев назад +3

      @@juanprosales Let's say it's a desktop app where you do have control of the environment. There's plenty of ways to get some level of access to a machine, and use other vectors to gain more.

  • @MoZaKx
    @MoZaKx 7 месяцев назад +37

    well, I believe that the guy who created this example simply forgot to change int to string, and that his idea was just to say to people to use parameters. But I agree, he should be more careful in public places.

    • @QwDragon
      @QwDragon 7 месяцев назад +1

      Actually I thought that the first query will fail, because it pases unused parameter. The first code was purely written. But if I want very much, I can make it vulnerable.

    • @anarchoyeasty3908
      @anarchoyeasty3908 7 месяцев назад +6

      @@QwDragon No you can't make it fail. It accepts an integer. You cannot pass ' ; or anything else required to do sql injection in via an integer. The dotnet type system will crash before that sql query is ever even generated.

    • @zpa89
      @zpa89 7 месяцев назад +1

      You don't forget to change ints to strings if you can code well enough to host a newsletter. Int and string are totally different things. You don't occasionally forget that your car is not a refrigerator if you know what a car or a refrigerator is.

  • @LukaszLech
    @LukaszLech 7 месяцев назад +23

    Parameterization is important for another reason. In MsSQL (and in different databases it is similar, I suppose): it makes SQL reuse the execution plan, which is way more efficient.

    • @megaporky
      @megaporky 7 месяцев назад +3

      Give that man a cookie. This is main reason for parameters rather than plain strings. It also very helpful when you need to run same sql query multiple times so you just change parameter value and not whole command string.

    • @TheMonk72
      @TheMonk72 7 месяцев назад +1

      True... but only when you're performing the same query multiple times. 99% of my queries don't hit the query cache. I know because I had this exact discussion a few years back with a colleague and we wasted a couple days instrumenting and studying logs to prove a point. In performance critical cases where you're repeating the same queries frequently it makes a difference. That accounts for a tiny fraction of the code I actually write, and I'm quite capable of identifying those cases. The rest of the time I get no advantage from parameterized queries... but my ORM handles them for me anyway.

    • @megaporky
      @megaporky 7 месяцев назад

      @@TheMonk72 Not sure what kind of area you working on. Maybe data analytics? As for any regular business solutions where 90% of actions are showing stored information by set of criteria's like ID/Tenant/Region it's quite common that most of the queries are very similar since people view similar data, just different ids/tenants or date ranges. Fact that you have 99% of time different queries is a bit strange

    • @davidmartensson273
      @davidmartensson273 7 месяцев назад

      @@megaporky I have had similar experiences as @TheMonk72, it all depends on what data your working with and what use case it is. In our case we had data from thousands of customers in the same DB and some customer had only a few lines, others had hundreds of thousands of lines. And a query plan built for a customer with 3 lines will be quite different than one built for 100 000 hits. At least when you then have joins on that first table, maybe multiple levels of joins.
      A bad query plan can in that case result in queries running for a minute instead of a few milliseconds.
      There are multiple ways to solve this depending on what DB your using but in MS Sql you have "sql server optimize for ad hoc workloads" which will avoid to specific optimizations in the query plan and instead opting for plans that should render average performance. The idea is that you avoid the very long running cases while sacrificing some performance on the fast cases since the fast cases will not be as noticable if the take twice the time while a heavy query taking 100 times longer will be a problem.
      Another solution is to actually avoid parameterization for specific values like in our case, customer ID, we interpolated that into the query resulting in all customers getting their own set of query plans. This worked since not all customer was online every day. If all had been online every day that approach would have polluted the query case to the point where it would no longer be useful.
      A third option is to add a statement to force a new query plan on every query rendering the query plan cache useless but in some cases that could still be the better option.
      But none of these should be the first choice, only when you know you have a problem and that the problem is sub optimal query plans should you start experimenting with there more arcane solutions :)

    • @wesplybon9510
      @wesplybon9510 7 месяцев назад +1

      Ashamedly, just a couple weeks ago I learned this is also true for dynamic sql strings executed using sp_executesql. Use parameters for values that will change frequently.

  • @andrewjosephsaid788
    @andrewjosephsaid788 7 месяцев назад +2

    I remember Jon Skeet's blog post "The BobbyTables Culture" where he created a contrived example where integers could cause SQL injection.

  • @rauberhotzenplotz7722
    @rauberhotzenplotz7722 7 месяцев назад +16

    In Germany, we say "dangerous half-knowledge". Nonetheless i would use a parameterized query even for the integer argument to have a constant query string for various performance reasons. You also have to think about other effects: What if the conversion to string introduces thousands separators or other formatting artifacts? The tip from LinkedIn is not wrong, but the reason is vague.

    • @tareksalha
      @tareksalha 7 месяцев назад

      totally agree, for high load API's the performance gain with query parameterization is dramatic.

  • @scottvercuski8993
    @scottvercuski8993 7 месяцев назад +45

    Have to say I absolutely LOVE this code cop series. There's SO much bad advice out there. I'd love to see a rating system for advice but that would also be problematic since there are an infinite number of opinions on code so a rating system would be difficult. Thank you for keeping this series going.

  • @UnnDunn
    @UnnDunn 7 месяцев назад +3

    Oh Bobby Tables, is that you? 😂

  • @BigPoleTightHole
    @BigPoleTightHole 7 месяцев назад +30

    I thought you were about to blow my mind by explaining how injection is done with an integer parameter. haha. That's what I get for not having coffee yet.

    • @nickchapsas
      @nickchapsas  7 месяцев назад +22

      Funny you say that, I actualy tried to see if there was some weird way I could manipulate an int to the point where it could cause an issue, but I had no luck :'(

    • @arjix8738
      @arjix8738 7 месяцев назад

      who knows, if it was C/C++ it could be undefined behaviour

    • @zadintuvas1
      @zadintuvas1 7 месяцев назад

      @@nickchapsas some cultures (like Swiss German) use ' for a thousands/group separator, but .NET does not seem to use it when converting integers nor decimals by default.

    • @NickSteffen
      @NickSteffen 7 месяцев назад +3

      @@zadintuvas1 Yea but that only matters when converting strings to ints though… If it’s already an int, it’s literally just 32 bits… the only possible variation is the endianness which is hidden by the runtime unless you are doing some weird stuff.

    • @QwDragon
      @QwDragon 7 месяцев назад

      Want to make integer vulnerable? Add this line somewhere:
      System.Globalization.NumberFormatInfo.NegativeSign = "1' OR '1'='";

  • @asdfxyz_randomname2133
    @asdfxyz_randomname2133 7 месяцев назад +3

    I really hate that people still think that input sanitation is a real solution to sql injections.
    Don't get me wrong, it can be, but it's not a good solution if you want to be sure.
    Instead, people should understand that the problem with sql injections is, that the user input can influence the parsing of the query, because adding the user input happens before the parsing of the query.
    And the obvious solution to that is, that you invent a way to parse the "unfinished" query before the user input is added to is, and that's exactly what parameterization is.
    Now, there are situations when you cannot do that, where the query actually dynamically depends on the user input, but not in a way that can be exploitet by the user.

  • @antonzhernosek5552
    @antonzhernosek5552 7 месяцев назад

    Thanks, Chap. This was actually IMO the best video in the series because it highlights something very fundamental whereas the first ones were more about syntax

  • @willhunt3000
    @willhunt3000 5 месяцев назад

    as usual an enlightening and useful topic and presentation - great job, Nick.

  • @clashclan4739
    @clashclan4739 7 месяцев назад

    I'm not going to skip ads for this series. Thanks, Nick. Love your content.

  • @user-zp3th3tj8k
    @user-zp3th3tj8k 7 месяцев назад +1

    Coming from back in the day of Classic ASP, SQL injection was a much bigger issue. Plain old ADO was capable of parameterized queries, but old legacy "do it fast" code was often guilty of ugly string concats. As you demonstrated, once C# as a statically typed language came on the scene, some of those concerns were indirectly addressed. Ultimately using parameters to properly escape and sanitize values was the right answer.
    Appreciate you going through and demonstrating how the attack actually works. Not everyone has seen the horrors of what an adhoc string concat query can do to your database.

  • @SerafimMakris
    @SerafimMakris 7 месяцев назад

    One of the biggest problems with these posts on social media is that they confuse beginner programmers, and as a result, they don't know what is right and what is not. Because a beginner who sees a post on LinkedIn assumes it is correct since they believe it is on a professional platform and no one would write nonsense there. In the past, we had it with forums and blogs, now with posts on LinkedIn and newsletters. Very good work, keep it up.

  • @marvinjno-baptiste726
    @marvinjno-baptiste726 7 месяцев назад +1

    Your third db entry made me chuckle, a proper oldskool LOL 😂

  • @djenning90
    @djenning90 7 месяцев назад

    Nicely presented and great examples, thank you!

  • @Isr5d
    @Isr5d 7 месяцев назад +1

    parametrized queries are very useful, not only for preventing from SQL injections; but it also gives the ability to store them as constants, which reduces memory usage and increase reusability. Saves you headache from type conversions (CLR to DB), and it can also help the DBMS Query Optimizer to create a one query plan, and reuse it for that query. (using string interpolation will create a query plan for every value for the same query, which is something you don't want).

  • @WDGKuurama
    @WDGKuurama 7 месяцев назад

    I loved when you made the third newsletter 😂

  • @ChristianHowell
    @ChristianHowell 7 месяцев назад +1

    But after two decades of corporate development, I've noted that managers need an intervention or something... 3rd party frameworks have value in limited scenarios and hiring better people is the correct path, not more mediocre slackers...
    I have horror stories about REALLY BAD software management...

  • @mynameisshadywhat
    @mynameisshadywhat 7 месяцев назад +2

    If someone can inject SQL into your code through a C# int parameter then you have bigger issues than that.

    • @davidmartensson273
      @davidmartensson273 7 месяцев назад

      Well, the only way that comes to mind is if you manages to inject custom type casting for ints, but if you gain that kind of access to the code nothing is safe anyway ;)

  • @terjeber
    @terjeber 7 месяцев назад

    You should also say something about why you want to use parameterized queries even when SQL injection is not possible (with an int parameter)
    When you pass a SQL statement to a database it will (have to) compile that SQL statement and create an execution plan for it. If you use string interpolation for the query, the database will not be able to cache the execution plan, so it will do a compile of the SQL statement each time. With a parameterized query the database will cache and re-use the execution plan even when the parameters changes, which means that subsequent queries (even with different parameters) will be significantly more efficient.

  • @shaihulud4515
    @shaihulud4515 7 месяцев назад

    English is not my first language, but I'd say I can speak and understand it quite decently. Nick is often a bit fast for me, but I can usualy keep his pace. Now, the Code Cop series is a whole different story: you immediatelly realise when some "advisors" really piss him off - it's like fast forward, and he's breaking the sound barrier of talking. Even my girlfriend can't keep up with him!
    Aside from this: the content is so good - I am a happy subscriber!

    • @anotherinternetaddict
      @anotherinternetaddict 7 месяцев назад

      I always listen to him on double speed. Native English speaker. I don't think he's too fast. You can always set the speed to 0.75.

  • @MatthiasFuchs
    @MatthiasFuchs 7 месяцев назад

    Great explanation!

  • @josda1000
    @josda1000 7 месяцев назад

    I actually learned something, and learned i was conflating two ideas as well. Thanks for the clarification.

  • @lorcaranr
    @lorcaranr 7 месяцев назад +2

    There is also a lesson around least privilege security. I have seen systems where the application could make schema changes, like drop table :(

  • @istovall2624
    @istovall2624 7 месяцев назад +1

    Bobby Tables would be VERY upset.

  • @nelsonoliveira5374
    @nelsonoliveira5374 7 месяцев назад

    Would have loved to see an example of exfiltrating data from another table, just to drive it harder into peoples minds that no table is "unimportant" and care is always required

  • @pergardebrink
    @pergardebrink 7 месяцев назад +4

    Using stored procedures to protect against SQL Injection is something I've heard before, but you can still be vulnerable, and I've seen many live examples of that when developers are concatenating strings in T-SQL, meaning that they do proper parameterization from the C# code, but then take those parameters to generate SQL without parameterization before running sp_executesql.
    I think using stored procedures actually makes you more likely to introduce SQL Injection attacks since you are limited to what T-SQL offer and you many times need to resort to string concatenation and forget about using parameterization for sp_executesql. And I'm not sure security scanning tools like SAST will be able to help (like they can with C# code) since you might store your T-SQL code somewhere that the tool won't find it..

    • @TheMonk72
      @TheMonk72 7 месяцев назад

      I'll sacrifice efficiency to avoid sp_executesql. It's far too easy to abuse and can still be broken.

    • @RomanSoldier13
      @RomanSoldier13 7 месяцев назад

      You're not wrong, but stored procedures usually offer the same protection as parameterized queries. In my experience using sp_executesql will result in insane code review scrutiny, but I guess other places might be different, or some crazy use case. But generally "use stored procs" advice is mostly solid, just another approach to the same problem, with its own downsides / gotchas

    • @tareksalha
      @tareksalha 7 месяцев назад

      sp_executesql uses the same mechanism of parameterizing the execution as with the simple query. Thus, it is equally safe. The old advice to use it instead of simple queries is not about security, but rather about abstraction. With procs in place, your database tables can evolve, while still keeping your application code the same. You can implement custom database level security, etc...
      As you said, many developers don't know shit about how to properly deal with or even talk to the databases and thus db admins started to revoke them direct access to data.

  • @RupOase
    @RupOase 7 месяцев назад

    Verbatim strings are useful when one of the supported SQL engines is SAP HANA, but you still need to make the query compatible with the plain old MS SQL

  • @CharlesBurnsPrime
    @CharlesBurnsPrime 7 месяцев назад +1

    The correct way to handle this situation is NOT merely to parameterize queries, as the LinkedIn author awkwardly stated, it is to use a database account that cannot modify data in the first place. Why should a query ever use a DB user that can alter data?

    • @xXxRK0xXx
      @xXxRK0xXx 7 месяцев назад

      Yep this is the correct answer

  • @davidmartensson273
    @davidmartensson273 7 месяцев назад

    I have actually found cases where bit or integer fields actually can benefit from string interpolation or concatenation instead of parameterized.
    This can happen if the database is very unbalanced between the different values. Since a parametrized query shares the first query plan created, if that plan is created based on an edge case value that for example only matches one or a few lines (the opposite is also bad but usually not ad bad), another query later that matches several thousand lines, the query plan can be very bad for the second query taking many seconds or even minutes to run instead of fractions of a second if the query plan is built based on a query with multiple matches.
    If you interpolate that value it will instead result in different query plans each optimized for the number of hits for that value. If you have very many different values this can still be a problem since you pollute the query cache and in that case you might need another approach, like identifying the different groups of values and deliberately create different queries, or maybe ad an extra field for the type of value that you then interpolate into the query to force different query plans.
    Now, this is probably quite rare in most cases so generally you should definitely use parameters and spare the generation of multiple query plans and gaining performance.
    But in a few cases we found that interpolating some specific values (it was always bit or integer fields in our case) we improved overall performance 10-50 times :)
    SO if you have a query that sometimes start taking a lot of time and other times is very fast despite using the same input data it could be that the query plan was created based on some edge case value that does not represent the normal queries.
    Or if you have a query that is generally fast but for one or a few values becomes very slow, then it could be worth investigating this and maybe interpolating some values can be the solution.
    As for having to deal with SQL injection, I had the benefit of getting introduced to it by a security expert that asked me what WOULD happen if he tried a query so I got to fix the problem before anyone abused it (at least so far as we knew at the time).
    That was some 30 years ago and as far as I know I have not created a vulnerable query since then ;)

  • @SacoSilva
    @SacoSilva 7 месяцев назад

    This one really grindededed my gears

  • @Blu3Souls
    @Blu3Souls 7 месяцев назад

    As an EFcore user I don't really think about sql injections. But I'm still glad I learned about them and how to avoid them in university.

    • @davidmartensson273
      @davidmartensson273 7 месяцев назад

      Yes, even if the tool have you covered, there is still ways to circumvent that protection and if you do not know about it and find those ways you can still cause sql injection :)

  • @Mr43046721
    @Mr43046721 7 месяцев назад

    I would also show what kind of request ultimately comes to Postgres in the Docker container, using the example of SQL injection (with try to delete data from table)

  • @bfg5244
    @bfg5244 7 месяцев назад +1

    The code example as it doesn't suffer of SQL injections YET. But, as code grows, some junior developer might step in and refactor this method (say, extract WHERE part following specification pattern), then them there is a place for human error. Explicitly wrapping input parameters to prevent the issue is what the SqlParameter made for and it makes it clear even for juniors.

  • @RomanSoldier13
    @RomanSoldier13 7 месяцев назад

    Beautifully demonstrated. Totally agree that this was irresponsible advice, being misleading or imprecise when it comes to this stuff is a big deal, SQL injection is a real thing in existing production codebases. You have to understand it and know what you're doing when remediating it. Junk examples like this are not helping.
    Thank you for calling this stuff out and showing a correct approach

  • @alfonsdeda8912
    @alfonsdeda8912 7 месяцев назад

    Hi Nick, but in any case the validation should be done either in presentation layer and in data access layer?

  • @davidghydeable
    @davidghydeable 7 месяцев назад

    Yep, I had to fix a SQL injection issue that was raised by a pen test. It's what happens when you let front end developers write C#.

  • @zadintuvas1
    @zadintuvas1 7 месяцев назад +10

    To make the example exploitable you need to convince some country to adopt base85 or something like that for representing integer numbers and then .NET to implement that change when converting integers to string under that culture. I'd image it would be pretty hard to do in practice.

  • @ibrahimhussain3248
    @ibrahimhussain3248 7 месяцев назад

    Security newsletter!!

  • @handlez411
    @handlez411 7 месяцев назад

    Had a laugh at this one. Thanks for all the videos :)

  • @hhcosminnet
    @hhcosminnet 7 месяцев назад

    So tired of seeing all these clowns on linkedin sharing nonsense.
    They are not able to give good examples even. They don't understand how to parameterize a query. And yes, query injection is not something to joke about. One could delete all your data, or worse: steal it. One could discover the whole dB schema using this. If it's a shop then someone could just change prices. Not looking to give you ideas, but to make you aware of the consequences of not understanding what you do.
    Thank you for this series.

  • @DjDanny32
    @DjDanny32 7 месяцев назад

    Stored procedures (done properly) are another usedul layer to help prevent SQL injections. Even if someone was able to inject something, in theory the user you're running the SP as wouldn't have access to SELECT from a table or DROP one

  • @vulcwen
    @vulcwen 7 месяцев назад

    It's still good advice to always parameterize things if you can, for performance reasons and to prevent you/other devs from making mistakes if the API changes. But yeh, there is no actual in the moment risk of an SQL injection for putting an integer straight into the query.

  • @ErazerPT
    @ErazerPT 7 месяцев назад

    This one kinda got me because i was homing in from the go on the parameter being an int and my brain was trying to come up with a scenario to turn that into injection. By the time it got to the description lightbulb went on and "oh, he messed the example...lol". Anyway, call me paranoid but i do integrity checks on my methods, pass as parameters to stored procedures and the SP checks it's inputs because... dumb mistakes happen and dumb people happen even more. And add sane constraints into the table definition, can save you a lot of headaches.

  • @FirstLast-vz8lt
    @FirstLast-vz8lt 7 месяцев назад

    Smirked reading comment section. Parametrization is not a silver bullet and most devs blindly rely on it without a second thought. Ofc it's fine and dandy in a simple query inside a value of where clause. Try it on a column name or table name or custom producere name/arguments, nested subqueries etc. Alot of db-drivers/orm/you-name-it can't properly handle such scenarios thus leading to SQLi.
    Don't get me wrong you definitely should use parametrization but still pay attention at your exact query. Remember parametrization purpose is not to create dynamic queries. It's to separate query and data.

  • @ArgeKumadan
    @ArgeKumadan 7 месяцев назад +1

    as soon as i see the code snippet, i said wait, this is not injectable.

  • @elka2677
    @elka2677 7 месяцев назад

    I have to say that I see this as an example of a wider problem that has annoyed me for decades. Practical examples in software engineering books or online that are often shoddy, clearly don't compile/execute, get tested and are frequently misleading at best. Even some of the best, "every programmer should have one" books are horrific in this regard. My advice is always test everything and understand the problem you're trying to solve before accepting advice or a solution blindly. Even the best programmers amongst us make mistakes.

  • @codewkarim
    @codewkarim 7 месяцев назад

    We need another channel for Chap Nicksas please.

  • @pedrofpsimoes
    @pedrofpsimoes 7 месяцев назад +1

    The funny thing is that the wrong part is not exploitable, and the author of the post doesn't really understand what SQL injection really is 🤣 They need to look into "Exploits of a Mom" featuring little Bobby Tables. 😎

    • @phizc
      @phizc 7 месяцев назад

      Little Bobby "Tables" is the reason I remember how SQL injections work. It's unforgettable. 😂

  • @okmarshall
    @okmarshall 7 месяцев назад

    The original poster definitely saw something about concatenating being bad, but didn't realise that it applied to strings (which could be anything) rather than ints. No idea why someone would then share it as if they are a genius saving us from SQL injection woes...

  • @reikooters
    @reikooters 7 месяцев назад

    Problems with the first snippet
    1) text to int implicit type conversion can cause performance issues
    2) database server can't reuse query plans when the value is hardcoded in the sql query string
    3) select * instead of explicitly specifying only the required columns

  • @kondziossj3
    @kondziossj3 7 месяцев назад +8

    It would be good example if they would talk about query plans (each integer would generate a new plan) but not as SQL injection as Nick said :D
    Of course... it depends for what purposes you made this query, because it can also evolve as parameter sniffing, but most of the ppl don't even know what this is :D

    • @nickchapsas
      @nickchapsas  7 месяцев назад +14

      Yeah query plans is a whole different thing, and actually it's a really good topic for a video! I'll add it to the list

    • @billy65bob
      @billy65bob 7 месяцев назад +3

      Depends on the database engine I guess.
      But for SQL Server, a query that simple would get caught and adjusted by 'Simple Parameterization'.

    • @StuartQuinn
      @StuartQuinn 7 месяцев назад

      ​@@billy65bobI was about to say that, been in the engine for at least 20 years. I think it was introduced in SQL 2000, so that's quite a while!

    • @TheMonk72
      @TheMonk72 7 месяцев назад

      ​@@nickchapsasplease do... but I'd love it if you point out that in a lot of cases applications don't actually repeat queries often enough for it to be worth worrying about. It's important if you're doing repetitive queries, but basically irrelevant if not.

    • @kondziossj3
      @kondziossj3 7 месяцев назад

      @@billy65bob as everything... has pros and cons. There is no silver bullet for everything

  • @jwbonnett
    @jwbonnett 7 месяцев назад

    What are you doing to get your program to start so quick? Mine typically takes a while to build no matter how small the program or how much resources I have on my machine! But as you are changing code and then running, you'd have to go though that build process right? So it would normally be slow...

  • @1DwtEaUn
    @1DwtEaUn 7 месяцев назад

    I get emails all the time from scam "security researchers" saying: Your app has SQL Injection issue, you send us 5000US we tell you fix.
    Half the time they try a DDoS as their proof of an issue.

  • @br3nto
    @br3nto 7 месяцев назад

    6:29 I get what you’re saying, and get that the example using int is a bad choice, but the advice is still good for a couple of reasons. First, if some future dev who doesn’t know better decides to copy and repurpose the safe interpolated or concatenated code (because of the int) for another query that takes some other data type like a sting… boom! SQL injection! Just because in one place it is safe to use interpolation or concatenation doesn’t mean you should do that. Second, each time a dev looks upon that method, they have to reevaluate “Is this safe”? And go through the mental gymnastics to validate that it is. When instead, if it has parameterised placeholder, it’s obvious. It’s better to err on the side of caution. It’s better to make the devs job easier and reduce the cognitive load. Think of it as a code standard that should be enforced by a linter. Just because you can, doesn’t mean you should. It’s poor workmanship.

  • @CharlesBurnsPrime
    @CharlesBurnsPrime 7 месяцев назад

    I can certainly imagine a situation in which a developer that does understand security wrote a quick LinkedIn post, but the code that he happened to quickly copy used an integer. In general, the core of the advice is to parameterize queries, which of course is a sound strategy.
    That said, it is much more likely that you are right and that he has no clue about security because in my experience that is true for almost 100% of developers. Still though, the core advice about parameterizing queries is sound.

  • @Iingvarka
    @Iingvarka 7 месяцев назад

    Nice one. SQL injection in a nutshell

  • @mariusgstanescu
    @mariusgstanescu 7 месяцев назад

    "If you're not a security expert, don't try to provide security advice!" - goes on and provides security advice :))
    Just nitpicking, don't worry, the content is great. :)

  • @ErikBongers
    @ErikBongers 7 месяцев назад

    I've seen simple php websites where user input is directly used for a search. These were small tools for internal usage. A small pdf generator, for example. I think the full select statement may have been built on the client side. I didn't have the heart to tell it to the (non-professional) developer.

  • @rafazieba9982
    @rafazieba9982 7 месяцев назад

    Fortunately even though the example and the explanation are bad the advice is good and (almost) fully correct. It's worth mentioning that string interpolation can be a good way to prevent SQL injection if such parameter is handled correctly by the method accepting it. EF Core has a method FromSql and another one FromSqlInterpolated. Both are doing it the right way. Be careful though. If you make a string out of it yourself before calling the method you will end up with a potential for SQL injection. Using parameters is always safe.

  • @muhamedkarajic
    @muhamedkarajic 7 месяцев назад

    Nick said it.

  • @lonniewatson9109
    @lonniewatson9109 7 месяцев назад

    Yes In multiple cases analysis tools employed by or security wonks flagged things in our applications as injectable, This was followed by numerous meetings with those same wonks to justify why they were not exploitable. This was followed by a paper trail of accepted "MEAT SPACE" mitigations. Which of course lasted until that same mindless engine that flagged the code initially was updated where the whole process started again. A process that continues to this day. So unfortunately, my team has had to go in and convert everything to using parameters in all cases just so we don't have to go through all this wasteful nonsense every few months... Endlessly frustrating to be sure... Kudos to you...

  • @acasualviewer5861
    @acasualviewer5861 7 месяцев назад

    I'm not sure about C#, but in some languages (like Java) the string parameterization is dependent on the database driver. Meaning that in some (rare) cases the driver could just concat the string together before sending it to the db, rather than going through the prepared statement step.
    My point is that proper prepared statement behavior is not NECESSARILY guaranteed. While using prepared/parameterized statements makes code more robust against SQL Injection, it may not always be sufficient.
    So you should also always check / cast your inputs as well. (Casting to an int, for example is great protection)

  • @stevenodlum4563
    @stevenodlum4563 7 месяцев назад

    One of the first jobs I had handled sql injection by rewrapping every parameter in single quotes. Didn't matter what it was, it would get another set of double quotes to 'prevent' the rampant drop table injection. They did a lot of things in a very... interesting way. Sometimes I wish I could go back to that position just to fix those problems with the experience I have now.

    • @davestorm6718
      @davestorm6718 7 месяцев назад

      Yeah, I made this mistake early on and was catching key phrase matches as well. This was thwarted when an attack was done via hexadecimal characters (2003). Another mistake people make is assuming an SP will save you from injection - it will not. You can do this via a text field and some gleaned info on the procedure itself (this is where dynamic sql needs to be checked and sql security settings locked down to prevent everything from drop/truncate/etc to emailing from the database - which should be completely disabled - ).

    • @leeroberts1192
      @leeroberts1192 7 месяцев назад

      @@davestorm6718 Any app or user should have just the rights/privileges required to do their task and no more. Also where possible what db tables they can use should also be restricted

  • @_JustBeingCasual
    @_JustBeingCasual 7 месяцев назад

    Sqlinjection with integers, nice

  • @Alex-gj2uz
    @Alex-gj2uz 7 месяцев назад

    I prefer drop table instead of returning everything.
    Some nasty person registered a sql injection as a company name and made some headache to the registration office

  • @blackenedsprite8542
    @blackenedsprite8542 7 месяцев назад

    The third entry in the database was fucking savage 😂

  • @brianpendell6085
    @brianpendell6085 7 месяцев назад

    So let me see if I understand what you're saying.
    The first example fails because we have an int passed in , and therefore no injection is possible. HOWEVER, if the argument was a string , and the call was not parameterized such that the string was directly added to the Sql command -- then that WOULD be an exploitable vulnerability. Is that correct?
    If so, I think this is far less of a problem than it's made out to be. While the specific example is poorly chosen because we're using an int, the code is not less safe because we follow the pattern set in the bottom #2 example. I argue we should still follow the #2 example anyway, even if it doesn't add much security, simply because I would rather developers understand one pattern that works every time as opposed to trying to write their methods cleverly because the additional boilerplate isn't strictly necessary. That way lies unnecessary code variation, and that way lies mistakes.

  • @jessegador
    @jessegador 7 месяцев назад

    I was laughing even before finishing the video because the guy who posted that one on LinkedIn knows nothing about SQL injection.

  • @cdarrigo
    @cdarrigo 7 месяцев назад +14

    Please do a video on ConfigureAwait

  • @AndreSilvaCardoso
    @AndreSilvaCardoso 7 месяцев назад

    I think the main problem would be other developers “reusing” the same code and not using parameters in other methods where the input was a string. So unless there was a very strong case for that method not using parameters, I would flag that as a potential problem

    • @davidmartensson273
      @davidmartensson273 7 месяцев назад

      I agree, as I mentioned in some other answers higher up I have had cases where interpolation of some specific parameters actually improved performance BUT we only did this AFTER having found that we had a performance problem and that it was due to bad query plans.
      I always use parameters when I write new queries and its very rarely necessary to do anything else.
      And even if interpolating an int is safe, unless there is some special cases for the field that triggers the mentioned edge case, parameters will still in thee majority of cases improve performance over interpolation.

  • @jeffreyhamans5647
    @jeffreyhamans5647 7 месяцев назад

    ever since i saw your first episode of copecop i wondered how long this example would come by. When i saw this article on LI i had a hard time not to laugh out loud because of how bad of an example the example code was, along with how the parameter code was still in the bad example. For me this was a clear example of OP creating content just for the sake of creating content interaction, without actually putting any sort of effort in. It is sad that LI has such a strong bias to content interaction creates content interaction rather than content quality creates content interaction

  • @wesplybon9510
    @wesplybon9510 7 месяцев назад

    Oh, yes... at a past company, 15 years ago at this point, we absolutely had issues with SQL injection. Our software's login page could accept and execute sql. The lead software engineer of our company demonstrated how to reset passwords for all our users' in one our staging environments. Well, he THOUGHT he was on one of the staging environments. At some point he got bounced to production and didn't realize it. So, yea... that was a fun day for support 😂 As far as we knew, WE were the biggest threat to our production security, so I guess that's good at least.

  • @jongeduard
    @jongeduard 7 месяцев назад

    I strongly feel that since we have all those AI code generators, we have a serious increase of people very new to programming who really believe they have suddenly become actual great engineers, while they just share randomly composed stuff without even trying to understand it themselves. 🧐 🤦‍♂

  • @cubbucca
    @cubbucca 7 месяцев назад

    I was watching just waiting for that semicolon.

  • @Velociapcior
    @Velociapcior 7 месяцев назад +1

    Hey Nick, I'm curious. Have some of the bad advise authors reached to you and give you some feedback etc? Or have you checked if after publishing video, they deleted their posts?

    • @nickchapsas
      @nickchapsas  7 месяцев назад +2

      I've seen at least 1 reaction and I think 1 deletion but no one has reached out directly to me

    • @Velociapcior
      @Velociapcior 7 месяцев назад

      @@nickchapsas what was the reaction if I may know? I'm wondering if the ego kicks in in those situations, or one is able to reflect on their wrongdoings.

  • @mohamedh.guelleh630
    @mohamedh.guelleh630 7 месяцев назад +4

    Please share more security tips and how to protect against malicious attacks

  • @T___Brown
    @T___Brown 7 месяцев назад

    @2:20 are you saying you are a security expert?

  • @shioli3927
    @shioli3927 7 месяцев назад +1

    While I agree that the advice is a bit odd because it´s not actually SQL injectable. The example might be flawed, but the core advice is good. Especially, when talking to 'not so great' devs I try to give them black and white answers to things when feasable. If they wanna dive deep great! But otherwise, you know. No harm done by doing parameterized queries everywhere. Personally working in a company that is like 50/50 mixed either programmers (meaning once that actually know what they are doing) and domain experts for the field that also program a bit. Without the latter the program would not be possible either. But it means a lot of the stuff they interact with has to be designed to be mostly idiot prove and often worked over by somebody else regardless. Black and white answers are flawed in programming but if the drawback isn´t really there (like in this example) they work out just fine and are followed more likely.

  • @ThekillingGoku
    @ThekillingGoku 7 месяцев назад

    Though to be honest, I haven't run a straight up SQL query from C# in lord knows how many years. It's mostly LINQ driven now (e.g. EFCore).

  • @Esgarpen
    @Esgarpen 7 месяцев назад

    Always leave IT/Cyber-security to people smarter than yourself - that's my LinkedIn Advice

  • @patxy01
    @patxy01 7 месяцев назад

    Sql injection is just the tip of the iceberg...
    Unfortunately, I see a lot of devs that think their code is safe because they're using EF.

  • @InfotechArchives
    @InfotechArchives 7 месяцев назад

    Man... That's damn true 🤔

  • @Bliss467
    @Bliss467 7 месяцев назад

    A bit of a stretch since the solution to injectable code you came to is the same one in the article. Tho I also easily identified why the bad example is not injectable, there’s no harm in convincing someone to always parameterize and never interpolate for queries. My biggest issue with the overly verbose way it was written….

  • @arekxv
    @arekxv 7 месяцев назад +1

    While I agree with you on this 100%, I still think the Linkedin advice is sound. And its mainly because of beginners and junior developers. You can have developers which try to be "smart" by doing the integer and saying "yeah there is no way SQL inject here!" and then later in the project some non-knowing dev changes that to a string because they didn't read the function far enough or understood it and bam, you got a SQL injection.
    Just because of this I would change the advice in this youtube video "Absolutely no matter what the reason is - use Parametized queries, please!". Don't make clever solutions, use proper standards :)

    • @nickchapsas
      @nickchapsas  7 месяцев назад

      The solution is correct but the starting point is wrong. When you give advice and you explain a problem, the way you present the problem needs to be accurate. If it’s not then the whole advice is bad because you don’t make clear what exactly you’re fixing

    • @davepusey
      @davepusey 7 месяцев назад

      I have to agree. By getting into the mindset of just using paramater-based queries everywhere AND NOTHING ELSE you can be certain there is no chance of accidental or intentional SQL injection anywhere in a codebase. Also, parameter-based queries allow for query-plan caching by the database engine, as the query itself remains the same while only the parameter values change. This improves execution time and database performance.

    • @blackenedsprite8542
      @blackenedsprite8542 7 месяцев назад

      The issue with telling beginners or juniors this is not that it's bad to parameterise stuff, it's that you're not explaining why properly. Without context they're doing it 'because that's how I do it' and never understand the broader implications, so maybe they don't pick it up in PRs (MRs), maybe they can't offer advice to someone, maybe they see it but gloss over it in existing code; because they don't know.
      Is linkedin the format for such explanations? I don't know, but if you can't make it so then don't write the post.
      This video is 12 minutes, 10 without an ad. It's not difficult to teach to someone.

  • @SXsoft99
    @SXsoft99 7 месяцев назад

    and they say PHP is not secure
    seems like the problem is between the chair and the monitor
    the first one for example is funny cause your method accepts Int variable, if you pass a string you have a runtime exception or whatever C# throws

    • @nickchapsas
      @nickchapsas  7 месяцев назад

      You can't pass a string when the method accepts an integer. The code just won't compile

  • @BierTrinka
    @BierTrinka 7 месяцев назад

    Atleast SQLInjection is a big topic since php with those dynamic non-strict type shit, where everything could be passed to everywhere. And that is, what i think is still the biggest target for such attacks, badly administrated webserver with less secured scripts on it ;)

  • @leonm8906
    @leonm8906 7 месяцев назад

    You can just expose your api with a db user who does not have the rights to anything they shouldn't be allowed to. For example your public facing api shouldn't have access to the user table unless it's specifically an authorization service. Note: this is my opinion I also am not a security expert.

  • @dYna77
    @dYna77 7 месяцев назад

    What is the Software you/he are/is using?

    • @phizc
      @phizc 7 месяцев назад

      Nick uses the JetBrains Rider IDE.

  • @tareksalha
    @tareksalha 7 месяцев назад

    the example is pretty bad, but the given advice to use parameterization is still valid in my opinion. I've seen too many young developers not adopting query parameterization, which on one hand is a huge benefit for database performance. Also imagine, if the app changes later and there will be a second method retrieving the newsletters by text search. What will a developer do? reinvent the wheel? No, he will just copy/paste the existing method and change the argument. boom!

  • @AmateurSpecialist
    @AmateurSpecialist 7 месяцев назад

    Usually, my preferred way of doing SQL injections is to add a double dash to the end of my injection, treating anything after that as a comment.

  • @evildaimyo3746
    @evildaimyo3746 7 месяцев назад

    Thanks! Дякую!

  • @EldenFiend
    @EldenFiend 7 месяцев назад

    When you copy paste an example from untyped PHP.

  • @dvanrooyen1434
    @dvanrooyen1434 7 месяцев назад

    Nick the code above would have pinged on a tool like checkmarx due to not using parameters and the input is not validated though technically int wont inject unwanted input

  • @davidpine7
    @davidpine7 7 месяцев назад

    Chap Nicksas 🤣🤣

  • @pyce.
    @pyce. 7 месяцев назад

    Oh shit, this looks like a Milan advice :D

    •  7 месяцев назад +1

      A quick google search, and it seems to be someone else, but he did comment on it. Did not see any repost from him though.

    • @pyce.
      @pyce. 7 месяцев назад +1

      Your searching skills are better than mine. Thanks for the info. 🙂

    •  7 месяцев назад

      @@pyce. I googled "sql injection tips linkedin C#", and clicked on "Images" tab. The second image was the original post (but it might be different for you).
      It also seems to have been a medium article with the same image in it, but I get 404 when i clicked on that one.
      I would also guess that since these are images, a reverse image search from a screenshot of the video should find it.