Authentication Bypass Using Root Array
HTML-код
- Опубликовано: 28 июн 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 
Наука
![Remble - Colors (ft. Mozzy & Stoneda5th) [Official Music Video]](http://i.ytimg.com/vi/KF-3AU9g9xg/mqdefault.jpg)
Thank you Eslam for sharing these details with us! Thanks to your openness we were able to learn some really cool stuff!
First. :3
🔥🔥🔥🔥🤝🐀🐁🐭👨💻
I also love the video editing style, so I want to replicate this, and expand on it.
My pleasure to work together bro 🎉
@@eslam3kl538 both the writeup link you gave on your tweet and the one LiveOverflow put in the video description do not work because they are missing a "." at the end of it, btw.
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!
yeah, really cool to reach out and follow up! y'all both learned something, and now we get to as well!
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.
what is prototype pollution, in detail, EH.
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.
ohhhhhh I did not realize that! good catch!
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.
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
@@msmyrk yeah, if it defaults to falsy it’s very bad
@@msmyrk it does default to true. LiveOverflow is setting it to false (0) in the video.
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?
Exactly!
I'd assume they didn't realize the query builder completely removes the where clause instead of checking "where username is null"
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.
@@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.
@@logiciananimal what is CISSP in detail.
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!
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
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.
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.
Yeee.
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.
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.
@@ryangrogan6839 Yeah, one thing I keep reminding myself is to never easily trust anything from the frontend, always validate and verify in the backend.
@@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.
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!
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
normally, I don't care whats in the object, I just take what I need, validate and move on.
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.
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.
It really is hard to believe that something like this is a thing in any enterprise application.
If these developers had evolved to the present times, they wouldn't be using php
@@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.
We're still implementing password authentication from scratch today, it's hopeless.
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
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!
Great Explanation LiveOverflow :) Thanks for digging into that and sharing the outcomes with the community. Also thanks for Eslam. GJ
Great video!
Realy liked the deep dive into trying to find out how the code worked even though you didn't have the sources.
Mind-blowing!!! Very very useful and knowledgeable as always!! Thank you! Thank you! Thank you! 🙏💜
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.
Awesome video. Big thanks to Eslam. Great job 👏
It was awesome. When I saw $where = []; I gave the same reaction when you saw that 'root' came from the xml formatting bug
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
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.
Haha. Thought the same.
Damn! The way he got deep into the issue is remarkable
i love you bro your content is gold !
please keep it up
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
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.
@@Reashu I said they are not hashing on the client side
@@patrick1020000 I know, I just wanted to clarify, since you mentioned it, that client-side hashing is hardly better than no hashing.
@@patrick1020000 first of all you should never query for a password as far as I know
I like this video. I hope to see more bugbounty ones 😁
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.
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.
why is it possible to add non standard parameters to orderby in an ORM? isnt an ORM database aware?
Great writeup! In depth and reproducing the issue
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
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.
Oh do you have a link for me where this is mentioned?
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.
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.
@@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.
thank you dude and plz don't stop
Videos like these make me wish I could give multiple upvotes,
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...
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
The request worked with an empty body so yes, that is correct
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
Why bother loggin in? Can't you just query the information you are looking for? 😄
5:12 "But that's just a theory, a coding theory!" that was 100% not by accident
MatPat reference :P
😌 thought so. Felt exactly like XML thing. Didn't thought that it could work by changing JSON key to anything.
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.
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 ^^
Really cool and interesting video, thanks!
This video is soo good showing both sides awesome
Excellent dig!! 👏
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.
Great video!
an extremely interesting find!
Hey Michael Cera, i didn't know you were a Sec Research guy, loved you on Year One!
Nice explanation, I was really confused before. lol
love your videos
Please do a video about the response manipulation topic you mentioned at 2:18
Awesome stuff! :D
Awesome video!
Great video
How it looks like => what it looks like
Server: "What is your username and password?"
Hacker: "Yes."
Server: "Welcome back, Sir."
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
Great! Thanks for sharing you knowledge ❤
Now thats some cool content
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.
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...
Validation is usually not up to the developer, but the product owners and they'll usually say no for budget reasons.
@@DoubleOhSilver This. I’ve seen this a lot.
Whoa, what an interesting rabbit hole
Great explanation
really interesting video!
Dude how does these things even come to you lmao what a GOD !!
I saw that pen spinning pen at 4:02 👀
He is a "pen" tester..
Game theory and bob ross references landed hahaha awesome
And there he is with more awesome, "free" educational videos
Shoutout to us. 😇
Thanks for you service
Thanks ahahah how did he not mention this in the article at first XD
Good thing my projects are so inconsistent I need to map the front-end keys to different variables anyway 😅
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.
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.
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.
@@gabiold Sending the plain text password over the wire isn’t really an issue as long as the connection is secured using TLS.
@@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.
:D I also love the video editing style, so I want to replicate this, and expand on it.
عاش يا اسلام، الله ينور 👍🏻👍🏻
Unpassend.
@@ParodieHecker-mobile do you even know what I am saying before saying that it’s inappropriate?
@@DigitalicaEG Yes, you are talking about god/Allah, but this video is about hacking.
@@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.
@@DigitalicaEG Hat trotzdem nichts mit dem Video zu tun
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.
That's why I just love Doctrine.
Love this kind of content. Why would a framework deliberately choose to only escape values but not keys tho?
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?
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
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.
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.
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)
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
very interesting! :)
Nice game theorists reference
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.
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.
So cool!
Very good i enjoy
in the note at 11:17 it says except when using a custom string. maybe "custom string" refers to the key?
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.
5:12 Nice reference to @FilmTheory :)
This is why validation is recommended, before you pass json further down your application
Interesting!
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
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.
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...
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.
🔥🔥🔥🔥
awesome awesome video
And that's why you sanitize, validate and use early returns.
Always always always sanitize data coming from somewhere else, regardless of if the source is trusted
I worked with codeigniter, I hope I didnt make this mistake. Though I wasnt the one who implemented the login.
What happens when the web developer and pen tester both have no idea what they’re doing? “rOoT aRrAy ExPlOiT”
I mean, baby steps. He did a better writeup, looks like he'sjust a beginner and is figuring stuff out.
He's just a newbie, he still found the issues. Understanding comes with wisdom which develops over time.
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
Even if they were escaped you could do like {"username":"blah", "userId":1}
HAHAH Our entire system is based on CI. Off to work I am!
Klassiker. First user is usually the admin.
"There are no accidents" - Master Oogway
I have seen this very same issue in another custom framework.
Wow LiveOverflow invented new header "Cintent-Type" (7:17)
The title of the video :D