Authentication Bypass Using Root Array

Поділитися
Вставка
  • Опубліковано 1 чер 2024
  • Lots of #bugbountytips get posted on twitter, but some of them are ... weird. Let's explore the technical details of one tweet to understand where this tip came from, why this tip was wrong, and eventually learn about the real underlaying vulnerability. This is a surprising turn of events!
    advertisement:
    Get my handwritten font shop.liveoverflow.com
    Checkout our courses on hextree.io
    Authentication Bypass Due to Empty Where Clause and SQL Injection in CodeIgniter liveoverflow.com/authenticati...
    Thank you Eslam for sharing the details with us!
    Follow Eslam on Twitter: / eslam3kll
    The #bugbountytips tweet: / 1526795822687346688
    Eslam's old post: infosecwriteups.com/authentic...
    Eslam's new blog: eslam3kl.gitbook.io/blog/bug-....
    Day[0] Podcast: dayzerosec.com/vulns/2022/03/...
    Chapters:
    00:00 - Intro
    00:41 - The bugbountytips Tweet
    01:21 - The Original Blog
    02:43 - Talking to Eslam about the Happy Accident
    04:36 - Digging Deeper
    05:39 - Researching Login Code with Codeigniter
    06:54 - Example Vulnerable Login Code
    08:08 - Improving the Writeup
    09:18 - Surprise SQL Injection!
    11:37 - Conclusion
    12:31 - hextree
    =[ ❤️ Support ]=
    → per Video: / liveoverflow
    → per Month: / @liveoverflow
    2nd Channel: / liveunderflow
    =[ 🐕 Social ]=
    → Twitter: / liveoverflow
    → Streaming: twitch.tvLiveOverflow/
    → TikTok: / liveoverflow_
    → Instagram: / liveoverflow
    → Blog: liveoverflow.com/
    → Subreddit: / liveoverflow
    → Facebook: / liveoverflow
  • Наука та технологія

КОМЕНТАРІ • 338

  • @LiveOverflow
    @LiveOverflow  Рік тому +401

    Thank you Eslam for sharing these details with us! Thanks to your openness we were able to learn some really cool stuff!

  • @rebane2001
    @rebane2001 Рік тому +322

    Neat explanation, I was so confused when I first saw you tweet about it - I figured it was going to be some weird prototype pollution or something. Props to you for reaching out and looking into this with Eslam instead of just writing this off as bs, and props to Eslam for updating the blog with this info too so others can make use of it!

    • @heeeyno
      @heeeyno Рік тому +1

      yeah, really cool to reach out and follow up! y'all both learned something, and now we get to as well!

    • @HenryLoenwind
      @HenryLoenwind Рік тому

      My first guess was that they implemented their auth function to put its result into the input data structure. One would think that would clash with the valid keys in there, but if the auth function is written as a transformer, replacing the parameters with the result, it would work.
      This is a common pattern in enterprise applications, where the same data structure needs to be given to a dynamic list of functions (e.g. supplied by plugins) that may all modify it. However, starting with an uncontrolled structure always is a bad idea.

    • @ahmedifhaam7266
      @ahmedifhaam7266 Рік тому

      what is prototype pollution, in detail, EH.

  • @DelusionalLogic
    @DelusionalLogic Рік тому +189

    At 11:17 you should read the note just below. The third argument tells CodeIgniter not to escape the "fields", those "fields" are the keys in the JSON object. You're passing 0 as the third argument, which is falsy.

    • @LiveOverflow
      @LiveOverflow  Рік тому +82

      ohhhhhh I did not realize that! good catch!

    • @msmyrk
      @msmyrk Рік тому +35

      Either that parameter should default to true, or they should have reversed the sense of it. Having to explicitly set a flag to protect against sql injection feels very "php-like" to me.

    • @unconv
      @unconv Рік тому +7

      You could still probably send "{id: 1}" and log in to the admin user (or any other user id) since the id field is probably valid

    • @henrym5034
      @henrym5034 Рік тому

      @@msmyrk yeah, if it defaults to falsy it’s very bad

    • @unconv
      @unconv Рік тому +22

      ​@@msmyrk it does default to true. LiveOverflow is setting it to false (0) in the video.

  • @dom1310df
    @dom1310df Рік тому +105

    I wonder whether the backend developer assumed all requests would be well-formed and coming from the frontend, and forgot that they could ever get malformed input?

    • @squ34ky
      @squ34ky Рік тому +3

      Exactly!

    • @92Hidden
      @92Hidden Рік тому +20

      I'd assume they didn't realize the query builder completely removes the where clause instead of checking "where username is null"

    • @airman122469
      @airman122469 Рік тому +14

      I literally had to remind my chief architect that was possible. He hadn’t even considered someone using curl or some other non-front end access point to the rest of the application. My mind was blown.

    • @logiciananimal
      @logiciananimal Рік тому +3

      @@airman122469 It is a very common thing I explain to a lot of developers - somehow people think that the browser is a security boundary, and then I show off Burp Intruder and jaws drop. I've even had security people have that reaction; some of them "intellectually were aware" but did not realize what it meant as they hadn't actually seen it demoed, just learned the fact for a CISSP or the like.

    • @ahmedifhaam7266
      @ahmedifhaam7266 Рік тому

      @@logiciananimal what is CISSP in detail.

  • @matheuswohl
    @matheuswohl Рік тому +7

    when I saw "ci_session" my mind went instantly to "wait, are they using codeigniter?" lol I used to code with that a few years back

  • @MetaflameDragon
    @MetaflameDragon Рік тому +7

    Hey LiveOverflow! Looks like you have either a typo in the link to the new blogpost, or the link was updated. It leads to a 404 website, and the actual blog post has a period at the end of the link.
    Great deeper dive, by the way! I really appreciate when any kind of bug is analyzed more into depth to find the actual root cause, rather than just shrugging off weird behavior and implementing a bandaid fix - this goes for all forms of programming, debugging, and pentesting. Finding the root cause can end up eliminating many more related bugs and prevent more of the same type in the future, while a bandaid fix just adds to the spaghettiness of code and ultimately it becomes undebuggable.

  • @Raham98
    @Raham98 Рік тому +26

    I must say, in all honesty, you are the best content creator for all things cyber security. I love how you showcase your thought process and really explain issues and concepts in a very clear and understandable matter. With every video you upload it gives me much motivation to stop being lazy and learn new things every day!

  • @mitchellmnr
    @mitchellmnr Рік тому +6

    As per the first example with the if statement, if you just did a not check on both those values and returned a 400, you'd be following basic practices and the invalid requests would have been mitigated. But for the sql injection, if you don't directly pass the json and actually validate the data, then bases are covered. Both of these things should be basic security practice for any API... Easy way to find badly written apps lol so nice find! Thanks for going to find more info about the tweet!

  • @flow5718
    @flow5718 Рік тому +15

    These three steps should be mandatory when accepting JSON input:
    1. Check if JSON is formatted correctly
    2. Check the existence of the keys you want
    3. Parse and verify the key values

    • @ahmedifhaam7266
      @ahmedifhaam7266 Рік тому +2

      normally, I don't care whats in the object, I just take what I need, validate and move on.

    • @dimitralex1892
      @dimitralex1892 11 місяців тому +6

      so it all comes back to simple validate and sanitize unknown inputs.... the very basic of IT security...
      by the way: the reason it could be benefitial to not only validate for required data but also checking if there is too much data is, that you can monitor possible bugs, but more important you can monitor attacks. if your application always sends username and password und now your app received a completely different payload this may be a very seriois bug in your application or someone is pentesting your app. and to know whether your application is under attack is a valie information, especially if you can live monitor the requests and outcoming results.

  • @JS-ii3rn
    @JS-ii3rn Рік тому +10

    I will always like your videos now. I like your style and how it makes learning security fun. I hope it makes it worth the time for you.

  • @xray8863
    @xray8863 Рік тому +50

    Directly using the $request->getJSON() in a query builder is a common mistake, I think. It's something similar to using $request->all() in Laravel where its considered an unsafe practice.

    • @ryangrogan6839
      @ryangrogan6839 Рік тому +8

      Its essentially not sanitizing data. Sure it might be valid json, but you must verify the structure of the json and the contents. This would not have happened if the data was verified before use.

    • @xray8863
      @xray8863 Рік тому +5

      @@ryangrogan6839 Yeah, one thing I keep reminding myself is to never easily trust anything from the frontend, always validate and verify in the backend.

    • @ryangrogan6839
      @ryangrogan6839 Рік тому +2

      @@xray8863 I would go a step further and not trust ANY data that comes from the outside, regardless of front or back end. If an attacker gets a foothold in any of your services, malicious json can be sent to clients, other services, etc. Always sanitize before you utilize.

    • @moczikgabor
      @moczikgabor 4 місяці тому

      Why this is not common sense amongst backend developers?
      I mean, it's the same as at home leaving your window next to the front door wide open as everyone supposed to go through the front door.
      Your backend is a public service and
      your front-end code is open-source, there is not even a security by obscurity element in the whole thing!

  • @NatteDweil
    @NatteDweil Рік тому +100

    It's amazing to me that, in 2023, this is still something programmers end up implementing themselves (and sometimes make mistakes in). Authentication should be a solved problem that frameworks have implemented completely and flawlessly. We really are still in the infancy of computing if this is still the kind of mistake programmers *can* make.

    • @fonzanedelungini
      @fonzanedelungini Рік тому +4

      It really is hard to believe that something like this is a thing in any enterprise application.

    • @Wyvernnnn
      @Wyvernnnn Рік тому +12

      If these developers had evolved to the present times, they wouldn't be using php

    • @Maxjoker98
      @Maxjoker98 Рік тому +12

      ​@@fonzanedelungini What does enterprise really mean? That it's "bespoke" or "better" in some undefined way, or that it's being developed by a single company for profit, probably closed-source? Because if it's the latter, I'm fairly certain that this is what causes those kinds of problems.

    • @thear1s
      @thear1s Рік тому +4

      We're still implementing password authentication from scratch today, it's hopeless.

    • @MsBukke
      @MsBukke Рік тому +8

      It is a solved problem and many frameworks have authentication built in and you can also decide to use an authentication provider(facebook or google) but in many projects i worked in there was a real need to customize authentication. And to be fair the bug found in the video is a bug that could happen anywhere really

  • @b1twis3
    @b1twis3 Рік тому

    Great Explanation LiveOverflow :) Thanks for digging into that and sharing the outcomes with the community. Also thanks for Eslam. GJ

  • @kemal6039
    @kemal6039 Рік тому +2

    It was awesome. When I saw $where = []; I gave the same reaction when you saw that 'root' came from the xml formatting bug

  • @e995a1ad
    @e995a1ad Рік тому +1

    Brilliant! I found the article yesterday when you tweeted about the upcoming video, but still couldn't make any sense of it. Weird that he's mentioning working with you, but still providing a completely different (and inaccurate imo) analysis.

  • @user-eb7dd4xo2n
    @user-eb7dd4xo2n Рік тому

    Great video!
    Realy liked the deep dive into trying to find out how the code worked even though you didn't have the sources.

  • @adyp487
    @adyp487 Рік тому

    Mind-blowing!!! Very very useful and knowledgeable as always!! Thank you! Thank you! Thank you! 🙏💜

  • @NeoInTheMatrix680
    @NeoInTheMatrix680 11 місяців тому

    Awesome video. Big thanks to Eslam. Great job 👏

  • @biz1M
    @biz1M 11 місяців тому

    Cool find, had a hunch straight away "root" was from messing with content mime types between JSON & XML and hoping the response would spit out some parser clues or possible XXE as you mentioned.
    1:58 Looking forward to this video. I generally use it lazily to see if clientside JS attempts to redirect me or expose endpoints I should look at manually. I know know, get gud at scraping JS for such endpoints.

  • @hacklearndaily
    @hacklearndaily Рік тому

    Damn! The way he got deep into the issue is remarkable

  • @Sn0wCrack
    @Sn0wCrack Рік тому +3

    Been doing PHP for a bit over 5 years now using Laravel mainly and I'd say from my experience at least, ensuring set column names don't come from user input, or if they do ensuring they're validated to a specific set of values, is a pretty common thing.
    While I've never particularly run into that issue, it has come up in some frameworks in the past, generally in relation to sorting when creating an ORDER BY clause in SQL.
    In Laravel 5.8 they added an explicit check to the orderBy function on their ORM and Query Builder that checked to make sure the direction parameter was either "asc" or "desc" as a security measure to protect users, and there's a warning in their documentation stating that you shouldn't take in user input for the order by function due to PDO lacking the ability to bind column names.

    • @ahmedifhaam7266
      @ahmedifhaam7266 Рік тому

      why is it possible to add non standard parameters to orderby in an ORM? isnt an ORM database aware?

  • @sutsuj6437
    @sutsuj6437 Рік тому +60

    5:12 "But that's just a theory, a coding theory!" that was 100% not by accident

  • @AskTheSloth
    @AskTheSloth Рік тому

    just remembred this video again. this format is amazing! would love if you would make it into a sieris. extremley interesting, straight to the point and learned alot :D

  • @pflasterstrips7254
    @pflasterstrips7254 Рік тому +6

    my first guess was that there is a JSON library where "root" is used to get the root of the JSON tree, but it's used in a way where it could also mean the propery "root" in the tree.
    The equivalent of naming a file ".." in a directory tree.

  • @xxabo7medxx517
    @xxabo7medxx517 Рік тому

    i love you bro your content is gold !
    please keep it up

  • @patrick1020000
    @patrick1020000 Рік тому +6

    Here's another "probably": The application is probably storing plaintext credentials. Think about it - if you're passing getJSON with a plaintext password into a getWhere, where is the hashing taking place? It's not happening on the client side based on the demo. If the app was hashing the password from the user, it would probably error out at that point rather than fall through

    • @Reashu
      @Reashu Рік тому +2

      Hashing client-side is almost as bad as not hashing:
      If someone pwns your database they can just write a client that sends in the hashes they stole without any further modification.
      Stolen credentials from other databases work in your app the same as before.
      HTTPS takes care of protecting the password in transit.
      The only thing you've protected against (compared to storing in plain text) is leaking a password that is used on another site. Not negligible, but not enough either.

    • @patrick1020000
      @patrick1020000 Рік тому

      @@Reashu I said they are not hashing on the client side

    • @Reashu
      @Reashu Рік тому

      @@patrick1020000 I know, I just wanted to clarify, since you mentioned it, that client-side hashing is hardly better than no hashing.

    • @nkazimulojudgement3583
      @nkazimulojudgement3583 11 місяців тому

      @@patrick1020000 first of all you should never query for a password as far as I know

  • @reseptivaras
    @reseptivaras Рік тому +46

    This is something that can happen in any framework and in any language. This isn't mentioned in most framework documentation because it should be common sense to not pass in the entire request to your database queries lol. Although I do that in my personal projects too... it's just such a simple way of making an API. most endpoints are one line of code. But never do that in user management sheesh

    • @sevret313
      @sevret313 Рік тому +13

      The Codeigniter documentation actually warns against this, but in a different user guide. The problem here and why it's not so obivous/common sense is that it does filter values so you'd expect it to filter keys too. And when you do half of the job you need to ensure to make it very clear to the developers that it does not do the other half of the job.

    • @LiveOverflow
      @LiveOverflow  Рік тому +13

      Oh do you have a link for me where this is mentioned?

    • @DoubleOhSilver
      @DoubleOhSilver Рік тому +6

      I disagree, it's reasonable assume it would also filter keys even if you're aware of the dangers of not doing so. Why have a framework if it's just going to cause you more trouble? If you have to go looking for the part where the docs warn you about this, then it's just not good. It should be clear as soon as you look up the usage.

    • @Wyvernnnn
      @Wyvernnnn Рік тому +13

      Stop trying to excuse the bad code by blaming the user. An ORM that doesn't sanitize parameters should get killed off ; getWhere() is supposed to sanitize the inputs, there's no reason it shouldn't sanitize the keys. If programmers like you were right we'd still be stuck in the 90s with SQL injections everywhere
      "Guardrails?! If the mason saws off his fingers it's because he was a BAD mason!"
      Cool opinion, my bandsaw has guardrails.

    • @Stdvwr
      @Stdvwr Рік тому +3

      @@Wyvernnnn I'm not a framework developer, but the reason of not sanitizing the keys is very simple: you can't. If the user can control the keys he can construct any query he likes without SQL injections. Instead of {"username": "user", "password": "pass"} he could literally send {"id": "1"} and probably log in as admin. There is no way for the framework to sanitize this because there is nothing to sanitize here.

  • @jonathanhoyos8191
    @jonathanhoyos8191 Рік тому

    I like this video. I hope to see more bugbounty ones 😁

  • @lake5044
    @lake5044 Рік тому +3

    Just like in hunting a bug, once you find something that works, you have to modify it to see when it stops working! So if "root":"blabla" works, I'd first start by "blabla2" and then by "root2", then by deleting it altogether, just to see if the code is even reading that. The initial conclusion really felt like superstitious thinking observed in pigeons associating a button that does nothing with whether the random food gets served or not.

  • @Athari-P
    @Athari-P Рік тому +9

    If the server passes raw JSON from the request into the query builder, there's no need for SQL injection. Nothing is stopping you from removing the password field and logging in with just a user name...

    • @edgars9581
      @edgars9581 Рік тому +1

      If they still have something like if (!isset("user") || !isset("pass")) {die;} then it wouldn't work, but that depend on if the dev implemented it

    • @user-fh7ki5bv5x
      @user-fh7ki5bv5x Рік тому +1

      The request worked with an empty body so yes, that is correct

    • @enterrr
      @enterrr Рік тому +2

      This! It's actually more powerful to send `{"username": "root"}` than just `{}` since this will ensure getting the root credentials vs hoping first row returned would be super user

    • @moczikgabor
      @moczikgabor 4 місяці тому

      Why bother loggin in? Can't you just query the information you are looking for? 😄

  • @qRaum
    @qRaum Рік тому

    Videos like these make me wish I could give multiple upvotes,

  • @SnakerDLK
    @SnakerDLK Рік тому

    Great writeup! In depth and reproducing the issue

  • @happyjester
    @happyjester Рік тому

    thank you dude and plz don't stop

  • @arili-eo7vw
    @arili-eo7vw Рік тому +11

    And there he is with more awesome, "free" educational videos

  • @LostMekkaSoft
    @LostMekkaSoft Рік тому +1

    hey, senior webdev here. i dont do much with php nowadays, because i value my mental health, but i worked on many php based projects in the past.
    to me personally, the auth bypass issue seems to be such a simple programmer error and so detached from the used framework, that it doesnt make much sense to include this in any framework-specific documentation. if the code really looks like this, it is just a really careless oversight and you cant include all of the ways to do things wrong. it would only be correct to blame the frameworks if they happen to have a kind of tutorial that already comes with this code as a starting point. in my php time i noticed that the php community is very prone to just copy-paste security relevant code without questioning it, so this might have lead to the issue.
    the sql injection issue is cool though, because it gives a clue on what ORM the devs used and potentially also what version. if the documentation of that ORM doesnt warn users that the keys are not escaped, then this is a pretty big oversight that can lead to many more devs to run head first into that trap ^^

  • @scorpian78
    @scorpian78 11 місяців тому

    Really cool and interesting video, thanks!

  • @KaishKhan1234
    @KaishKhan1234 Рік тому

    😌 thought so. Felt exactly like XML thing. Didn't thought that it could work by changing JSON key to anything.

  • @froozynoobfan
    @froozynoobfan Рік тому

    This video is soo good showing both sides awesome

  • @_Slaze
    @_Slaze Рік тому +4

    Please do a video about the response manipulation topic you mentioned at 2:18

  • @kevinalexander4959
    @kevinalexander4959 11 місяців тому

    Hey Michael Cera, i didn't know you were a Sec Research guy, loved you on Year One!

  • @ET_AYY_LMAO
    @ET_AYY_LMAO 11 місяців тому +1

    One of my proudest pwns was figuring out that like half of all LDAP integrations on the web can basically be null byte injected in the password field and you get login, I figured this out because 90% of ldap setups allows for "anonymous" logins in order to get basic info about the user, but many web devs just check if the user is authenticated, so if you can somehow initiate a login with a zero length password you get access.. Only downside is that you need to know a user name.
    I found this at a company I worked for about 8 years ago, and doing a little but of research it seems like we were not the only ones with that mistake. Especially on PHP sites, someone might check the length og $password, but if you can null byte inject the LDAP interface, then a password length is pretty useless.

  • @TanNguyen-cb2he
    @TanNguyen-cb2he Рік тому

    Nice explanation, I was really confused before. lol

  • @Parmaeham
    @Parmaeham Рік тому +3

    I find this insanely important, to somehow reverse engineer a technique where you are talking to, but have no profound understanding.
    For instance: I discovered how a Microsoft cloud application used WebSockets with GZIP instead of traditional HTTP requests to transfer data to its end users. This is how I now interpret performance issues with the server.
    Having an understanding of a framework is just utterly important to have a basic foundation of problem solving.

  • @browny99
    @browny99 Рік тому +3

    This is why you should validate this stuff at multiple levels, for example with validation middlewares and filters before even allowing it to go into a controller. Their client had 0 attention to detail...

    • @DoubleOhSilver
      @DoubleOhSilver Рік тому +3

      Validation is usually not up to the developer, but the product owners and they'll usually say no for budget reasons.

    • @airman122469
      @airman122469 Рік тому

      @@DoubleOhSilver This. I’ve seen this a lot.

  • @neilthomas5026
    @neilthomas5026 10 місяців тому

    Dude how does these things even come to you lmao what a GOD !!

  • @parthghughriwala6799
    @parthghughriwala6799 11 місяців тому

    Excellent dig!! 👏

  • @BertVerhelst
    @BertVerhelst Рік тому

    Great video
    How it looks like => what it looks like

  • @josephrissler9847
    @josephrissler9847 10 місяців тому

    Server: "What is your username and password?"
    Hacker: "Yes."
    Server: "Welcome back, Sir."

  • @Vsimpro
    @Vsimpro Рік тому +1

    Great video!

  • @kebien6020
    @kebien6020 Рік тому +8

    This reminds me of the sequelize ORM in NodeJS. They had this query syntax based in objects, almost looked like ast for a query, for example
    {"$or": [{"type": 3}, {"group": {"$lt": 5}]}
    Would produce
    WHERE 'type' = 3 OR 'group' < 5
    And applications tended to have a vulnerability when users were able to pass objects as user input. So eventually they replaced it so that operators have to be Symbol keys, which can't be created by the user.
    {[Op.or]: [{"type": 3}, {"group": {[Op.lt]: 5}]}
    Where Op is an object provided by the library containing a bunch of Symbol values

    • @NeoInTheMatrix680
      @NeoInTheMatrix680 11 місяців тому +1

      Great! Thanks for sharing you knowledge ❤

  • @angelo5274
    @angelo5274 Рік тому

    an extremely interesting find!

  • @HTWwpzIuqaObMt
    @HTWwpzIuqaObMt Рік тому +4

    Now thats some cool content

  • @AbNomal621
    @AbNomal621 10 місяців тому +1

    This is quite simple. If you find a site which this login works please notify the owner that the developer should NOT be writing code for public consumption.
    One: line 20 SHOULD simply return an error if either username or password is missing. Ideally, it barfs if any other object is there. But minimally, you should validate required fields are present and ignore extraneous. I know your not a dev, but neither is the person writing the site under investigation.
    Second, taking parameters from the keys of the input is a for, of begging to be hacked. But if one doesn’t know better than the first issue, they don’t know what SQL injection is, let alone how to prevent it.

  • @jupp6976
    @jupp6976 Рік тому

    love your videos

  • @user-yo2rf4pf2s
    @user-yo2rf4pf2s Рік тому +178

    What happens when the web developer and pen tester both have no idea what they’re doing? “rOoT aRrAy ExPlOiT”

    • @ZeroPlayerGame
      @ZeroPlayerGame Рік тому +61

      I mean, baby steps. He did a better writeup, looks like he'sjust a beginner and is figuring stuff out.

    • @lmaoroflcopter
      @lmaoroflcopter Рік тому +30

      He's just a newbie, he still found the issues. Understanding comes with wisdom which develops over time.

  • @rafaelcoelho5226
    @rafaelcoelho5226 Рік тому

    Awesome video!

  • @maxsilvester1327
    @maxsilvester1327 Рік тому

    in the note at 11:17 it says except when using a custom string. maybe "custom string" refers to the key?

  • @thatonesecguy
    @thatonesecguy 11 місяців тому

    Awesome stuff! :D

  • @alfred.clement
    @alfred.clement Рік тому

    I also saw the same tweet on Twitter recently and I was scratching my head for a second...
    I'm curious about Twitter's algorithm now, considering that post was from a year ago.

  • @permissionBRICK
    @permissionBRICK Рік тому

    Whoa, what an interesting rabbit hole

  • @funil6871
    @funil6871 Рік тому

    Thanks ahahah how did he not mention this in the article at first XD

  • @TheRaysfan22
    @TheRaysfan22 Рік тому

    Game theory and bob ross references landed hahaha awesome

  • @ColoredIceberg
    @ColoredIceberg Рік тому +2

    Good thing my projects are so inconsistent I need to map the front-end keys to different variables anyway 😅

  • @picklypt
    @picklypt Рік тому +4

    I saw that pen spinning pen at 4:02 👀

    • @squ34ky
      @squ34ky Рік тому +1

      He is a "pen" tester..

  • @ArtemYakovlev
    @ArtemYakovlev Рік тому

    Great explanation

  • @rooot_
    @rooot_ Рік тому

    really interesting video!

  • @logiciananimal
    @logiciananimal Рік тому

    Sometimes when I have time in web app pentesting, I deliberately try "weirdly different". That's what fuzzing is in general, but I mean probes that are in a way unjustified that are sort of off the wall just to see if anything unusual happens. This is unsustainable and slow but can sometimes be interesting in results.

  • @tranquility6358
    @tranquility6358 Рік тому +2

    What really grinds my gears is that there is no mechanism to check the password in code. It’being done on the database level in the where clause. Now I know this is just an example liveOverflow used, but judging by the behavior of the target website it’s entirety possible they also use plain text passwords, which is another level of issues altogether. They should’ve used something like BCrypt or Argon2ID both of which are supported fully by PHP.

    • @moczikgabor
      @moczikgabor 4 місяці тому

      The plain-text password should not have left the client side in the first place.
      Hashing with a nonce would both prevent a stolen ciphertext replay attack and having to send the client data straight to the db without doing something with it.

    • @tranquility6358
      @tranquility6358 3 місяці тому

      @@moczikgabor Sending the plain text password over the wire isn’t really an issue as long as the connection is secured using TLS.

    • @moczikgabor
      @moczikgabor 3 місяці тому

      @@tranquility6358 Yeah, IF everything is working as intended, then it is not a problem. However, if something goes wrong, then it is a huge weakness. For example, if the server can be accessed on a non-TLS connection (due to configuration oversight, either now, or someone makes a mistake in the future) and an attacker can somehow forcibly degrade the client's connetion, then it can steal the password. Or, if an attacker can gain access to the user database, it can aquire everyone's password which can be used to impersonate people on other platforms as well. Serious issue, as we know that a lot of people DO reuse the same password on other platforms.
      Security is not one level. It is multiple layers on top of each other. If one fails, the others can still prevent or mitigate the problem.

  • @ahmedifhaam7266
    @ahmedifhaam7266 Рік тому

    moment I saw root, I knew it was XML XD, but what type of auth for which backend services?
    I feel like, first instinct for if root or random XML working would be to check if any parameters or empty object worked too.
    edit: watched more of the video, as expected, this is from php... codeigniter query builder.
    overall, interesting video, and thanks for sharing the journey.

  • @DigitalicaEG
    @DigitalicaEG Рік тому +1

    عاش يا اسلام، الله ينور 👍🏻👍🏻

    • @ParodieHecker-mobile
      @ParodieHecker-mobile Рік тому

      Unpassend.

    • @DigitalicaEG
      @DigitalicaEG Рік тому

      @@ParodieHecker-mobile do you even know what I am saying before saying that it’s inappropriate?

    • @ParodieHecker-mobile
      @ParodieHecker-mobile Рік тому

      @@DigitalicaEG Yes, you are talking about god/Allah, but this video is about hacking.

    • @DigitalicaEG
      @DigitalicaEG Рік тому +1

      @@ParodieHecker-mobile nope. the guy’s name is Islam in Arabic that’s اسلام
      and he’s Egyptian like me which I found cool as I’ve never seen an Egyptian featured on this channel before. In Egypt when we want to tell someone that he did a good job we say Allah Yenawar الله ينور which translates in meaning to good job shedding light on this. Next time when using Google translate understand that just because it says it can translate Arabic doesn’t mean it’s correct. The Arabic language is full of nuances and has a lot of dialects that stray very far from the traditional Arabic like the Egyptian dialect and slang expressions that to Google will make no sense. Keep that in mind.

    • @ParodieHecker-mobile
      @ParodieHecker-mobile Рік тому

      @@DigitalicaEG Hat trotzdem nichts mit dem Video zu tun

  • @inao-cz
    @inao-cz 11 місяців тому

    That's why I just love Doctrine.

  • @olivierlasne2346
    @olivierlasne2346 Рік тому

    Thanks for you service

  • @ReligionAndMaterialismDebunked

    :D I also love the video editing style, so I want to replicate this, and expand on it.

  • @jacob_90s
    @jacob_90s 11 місяців тому

    Interesting, that is not what I thought the cause would ultimately be.
    I thought it would be one of two things
    1) It was a bug where during deserialization there was an object called "root" that was being overwritten.
    2) There was a separate login page for admin users that wrapped the credentials in the root object as a signal to the authentication script that the user should get higher level access (yes I have seen that before).
    I'm genuinely shocked it was something that simple though. Sounds like another case of people just mindlessly copying code without understanding it.

  • @iLegendaryGraphics
    @iLegendaryGraphics Рік тому

    Love this kind of content. Why would a framework deliberately choose to only escape values but not keys tho?

    • @JJJMMM1
      @JJJMMM1 Рік тому +1

      I suppose because it's not sensible for a developer to inject unvalidated column names from a client straight into a query builder. I find it much weirder that the CodeIgniter docs talk about escaping the values, which indicates they are not using prepared statements. Does anyone know what the reasoning (or history) of this is?

    • @botondhetyey159
      @botondhetyey159 Рік тому +3

      I think because it's not really an issue if you are coding up the controller properly. You should set the keys by hand, not just throw whatever you receive in.
      Of course, no accounting for devs misusing your framework, and a warning about this in the documentation would be the minimum

  • @KaydotOrigin
    @KaydotOrigin Рік тому +1

    PDO (the most common driver for database communication in PHP) does not support binding to columns, you can only bind to parameters - passing any user input to any framework’s where clause (in the column field) will cause a SQLi - unfortunately, it isn’t well known as far as I’ve seen

    • @dave7244
      @dave7244 11 місяців тому

      It wild that parameterised queries aren't as well known as they are. In a lot of other languages you kinda have to do it that way or you really have to go out of your way to introduce an SQL injection vunerability.

  • @infosecStudio
    @infosecStudio Рік тому

    Shoutout to us. 😇

  • @Shocker99
    @Shocker99 Рік тому

    5:12 Nice reference to @FilmTheory :)

  • @brunoais
    @brunoais Рік тому

    I never had such kinds of mistakes.... I hope I continue that way. For me, these stick out like a sore thumb.
    If username or password are not provided, just jump out of the function with a generic error that either was not provided.

  • @jonathan-._.-
    @jonathan-._.- Рік тому

    turning parameters into save sql is pretty easy: you jsut work with prepared statments
    but as far as i know column named cannot be easily turned into prepared statement format
    (this is the main reason for why i build my database npm layer with a somewhat annoying querybuilder)

    • @henrym5034
      @henrym5034 Рік тому

      The npm mysql library uses ?? to escape column names. It is still a bad idea to supply the whole user controlled object to the query builder directly

  • @Zeldapedia01
    @Zeldapedia01 11 місяців тому

    Can you show the contents of the \Config\Services::request() method? I am having a tough time recreating your example

  • @eye776
    @eye776 11 місяців тому

    modifying the response is basically _"pray the client side code has oversights in it that will let you do things on the server you should not be able to anyway"_
    and unfortunately for the webmasters, it does sometimes work...

  • @notfaang4702
    @notfaang4702 Рік тому

    This is why validation is recommended, before you pass json further down your application

  • @tarakivu8861
    @tarakivu8861 Рік тому

    9:17 I mean.. usually with Automated testing this should also get detected? I would even write Unit-Tests, testing for such edge-cases.

  • @yiannissiantos127
    @yiannissiantos127 Рік тому +2

    There's unfortunately no uniform way to escape column or table names in SQL database implementations. MySQL and MariaDB use backticks, Postgres uses double quotes (which I believe is also the SQL standard way) and SQL Server uses square brackets. This seems to result in these sort of "injection due to unescaped column name" issues. To me it seems like something that needs fixing in CI to escape column names and table names depending on what DBMS is used.

    • @HenryLoenwind
      @HenryLoenwind Рік тому +2

      While that is true, I see the root cause more in the desire to make SQL builders as dumb as possible. Instead of querying the database for column names and matching the parameters to those (throwing an error for unmatched ones), they just convert the parameter into text. However, a builder like that is useless; writing the where clause manually is arguably easier than building the data structure to feed to it---and it involves a skill that will stay valid forever instead of being framework-specific. I much prefer to write select("select * from users where username=? and password=?", $username, $password) over select({username => $username, password => $password}, "users") oops, select("users", {username => $username, password => $password}) still wrong, select({username => $username, password => $password, table => "users"}) maybe?.

    • @gunnargu
      @gunnargu Рік тому +4

      Say it with me: DO NOT GENERATE QUERIES FROM USER INPUT. Use parameterized queries.

    • @yiannissiantos127
      @yiannissiantos127 Рік тому +1

      @Gunni the problem is that when you use a framework, especially one that says it cleans up your values, it's reasonable to expect that it also would escape the names as well. It creates a certain expectation from developers, and when it doesn't deliver (either by escaping names or explicitly warning that it doesn't), it will end up causing issues. This is especially true if other frameworks do actually escape names by default (most do AFAIK).

    • @gunnargu
      @gunnargu 11 місяців тому +2

      @@yiannissiantos127 I have simply learnt to not trust I prefer to use queries directly

    • @NeoInTheMatrix680
      @NeoInTheMatrix680 11 місяців тому +1

      Yeah, what kind of standard is sql? Everyone has their own fucking syntax for sql.

  • @neadlead2621
    @neadlead2621 Рік тому

    can I ask about the sql payload what is the \" in the payload I don't get it

  • @lierdakil
    @lierdakil Рік тому +1

    The moral of this story for a practising programmer is: always, always aggressively validate user input and return 400 Bad Request if anything is even slightly off. Passing user-submitted JSON directly into a query builder is a terrible idea. JSON schema is a thing that exists, so one could automate validation to a large extent, but even ad-hoc validation can be leagues better than nothing at all. And before someone says that "frameworks should take care of it", good point, but don't blindly trust frameworks to always do the Right Thing, either verify user data yourself or be absolutely sure your framework does it for you -- in my experience, most don't, or at least don't do a terrific job of it, because only you know what kind of data your application expects.

    • @PathOfDamn
      @PathOfDamn Рік тому

      I like to follow this mantra: don't blacklist faulty inputs, whitelist proper inputs.

  • @Lopoi
    @Lopoi Рік тому

    I worked with codeigniter, I hope I didnt make this mistake. Though I wasnt the one who implemented the login.

  • @scamchan4936
    @scamchan4936 Рік тому

    Very good i enjoy

  • @expandingsalad786
    @expandingsalad786 11 місяців тому

    So cool!

  • @outseeker
    @outseeker 10 місяців тому

    very interesting! :)

  • @parekxddd7697
    @parekxddd7697 Рік тому

    Nice game theorists reference

  • @ILsupereroe67
    @ILsupereroe67 10 місяців тому

    It seems to me the part where the keys of the array passed to getWhere() are not escaped IS a bug in CodeIgniter, not just a mistake made by developers using Code Igniter like the other cases

    • @LiEnby
      @LiEnby Місяць тому

      Even if they were escaped you could do like {"username":"blah", "userId":1}

  • @RoiEXLab
    @RoiEXLab Рік тому +10

    It's crazy how it's always PHP when such obscure issues arise. Not saying this doesn't happen in other languages as well, but the paradigm of "just pass us the data and we'll handle it" just always reminds me of this language.

    • @inebriatedhamster
      @inebriatedhamster Рік тому +3

      PHP powers most of the web, so it's going to be in the firing lines for this sort of thing a lot more. That said, there's literally nothing about this issue that is PHP specific.

  • @dealloc
    @dealloc Рік тому

    And that's why you sanitize, validate and use early returns.

  • @gownerjones1450
    @gownerjones1450 Рік тому +1

    HAHAH Our entire system is based on CI. Off to work I am!

  • @JustCode39
    @JustCode39 Рік тому

    awesome awesome video

  • @ryangrogan6839
    @ryangrogan6839 Рік тому +1

    Always always always sanitize data coming from somewhere else, regardless of if the source is trusted

  • @adarsh-chakraborty
    @adarsh-chakraborty Рік тому

    I usually return with 400 response when the username or password is missing from request body. Why go further

  • @michaelraasch5496
    @michaelraasch5496 Рік тому +1

    Klassiker. First user is usually the admin.