2019/05/15

Feature or Bug? Story of Laravel's bug






Không biết bao nhiêu người có cùng câu hỏi này với tôi?
Là một người tập tành đi Dev, thi thoảng kiêm thêm Ops, tôi khá tự tin có thể setup một thứ gì đó theo ý của mình. Kể cả dev cũng vậy, code không sợ bị bug, chỉ sợ nhất là bug logic, kiểu như cover thiếu trường hợp chứ mấy lỗi sanitize sai, injection này kia cho vào dĩ vãng được rồi.
Nhưng càng làm thì càng thấy sai :)
Đồng thời cũng là một nhà ngoại cảm bảo mật, khi code thì đương nhiên là sẽ dùng document và ví dụ cho dễ hiểu, chỗ nào tôi thấy có những điểm mờ ám là note lại ngay để tìm hiểu sau.

Lần này thì tôi tìm ra một cái feature nếu dùng sai thì thành bug :? Thế thì nó là feature hay là bug?
Laravel Unique Rule SQL Injection Warning
https://blog.laravel.com/unique-rule-sql-injection-warning

Nếu dùng Validation (hành động xảy ra khi user submit input):
Rule::unique('users')->ignore($request->input('id'))
Vấn đề là unique, developer mong là người dùng sẽ input 1 giá trị số, hay giá trị gì đó. Tuy nhiên tôi phát hiện ra là hoàn toàn có thể đưa tên cột vào.

Do tên cột, tên bảng thì PDO không hỗ trợ binding, Laravel cũng không làm gì hơn trong trường hợp này. Nếu developer muốn dùng từ người dùng thì cần phải tự xử lý input, và tốt nhất là không nên sử dụng gì từ phía người dùng.
Khi mà ghép chuỗi thế này, thì  id=1,column_name được chấp nhận.
Tổng hợp lại ta có:

  • Laravel không xử lý column name
  • Người dùng có thể chèn bất kì thứ gì vào column_name nếu họ được phép 

Lỗi ở (src/Illuminate/Database/Query/Grammars/Grammar.php) https://github.com/laravel/framework/commit/be1896cbeb2e413615fb61791101f8b199e1bf3d#diff-132870d3cbe60fc402aa1a0a8b47387c
Thực sự tại thời điểm hiện tại tôi cũng không nhớ là tại sao tôi lại đến được đây, có thể là debug, có thể là giác quan thứ 6 :D, Nhưng mà, cái dữ liệu cần nhập vào cần chứa '->' (không có dấu quote). Đó là tất cả magic.




Cái unique rule cũng được patch
https://github.com/laravel/framework/blob/90fc0babe1c854249c1750be9dfc6ab8f0937935/src/Illuminate/Validation/Rules/Unique.php#L66
Câu chuyện không dừng lại ở đây khi mà một thời gian sau tôi phát hiện ra là cũng có những thư viện có thể bị ảnh hưởng, tức là lỗ hổng ở đây không còn dừng lại ở mức unique, mà liên quan đến những thư viện dùng Illuminate/Database, ví dụ như https://murze.be/an-important-security-release-for-laravel-query-builder
Thế thôi :D

---------
billie eilish

---------
Maybe you don't care about my bullshit story above :v So I will give you a TLDR below.

I found a bug in Laravel that in some case could lead to SQL Injection.
Official statement of Laravel: https://blog.laravel.com/unique-rule-sql-injection-warning

Example Validation rule:
Rule::unique('users')->ignore($request->input('id'))
PDO doesn't support column, table binding, so Laravel too, and you should escape the column name if you want to use it from user.
What if user has ability to do more than just a id?
In this case, ignore is what user can insert. Everything is concatenation.
Then id=1,column_name accepted :D

In conclusion:

  • Laravel didn't process (validate, sanitize) column name
  • User can input what ever they want if they allowed
The bug is in (src/Illuminate/Database/Query/Grammars/Grammar.php around line 1120 in function wrapJsonPath). This time I don't remember how I got into that place, maybe debugging, 6th sense :D, but, the input need include '->' (dash and greater than). That's all the magic.

https://github.com/laravel/framework/commit/be1896cbeb2e413615fb61791101f8b199e1bf3d#diff-132870d3cbe60fc402aa1a0a8b47387c


The patch also applied in https://github.com/laravel/framework/blob/90fc0babe1c854249c1750be9dfc6ab8f0937935/src/Illuminate/Validation/Rules/Unique.php#L66 to make sure no unintended behavior exists.



At the moment I re-read this blog, I found out that, not only Laravel has been impact by this bug, but also other library rely on Illuminate/Database vulnerable version, not limited to https://murze.be/an-important-security-release-for-laravel-query-builder
That's all folks

------------
Timeline:
2019/03/18: Initial finding.
2019/03/18: Vendor response.
2019/03/18-19: Information exchange.
2019/03/19: Vendor release blog & update documents.
2019/05/16: Blog released.


Example PoC:


  • Validation rule: 'username' => 'unique:users,username,'.\Request::get('username'),
  • GET: ?username=demo&id=1,username->="'))or(sleep(5))--+-
  • Real SQL query: select count(*) as aggregate from `users` where `username` = demo and json_unquote(json_extract(`username`, '$."="'))or(sleep(10))-- -"')) <> 1)



Further reading (aka, I'm too lazy)
https://stitcher.io/blog/unsafe-sql-functions-in-laravel
https://shieldfy.io/blog/serious-sql-injection-vulnerability-in-laravel-query-builder/

No comments:

Post a Comment