btcpuertorico on Nostr: 👀 nostr:note1wd4ng2ete2sa7vyv9gpng9hh96e8axqgjhwtjsk6j0hcdz4jmdjs92hgt3
👀
quoting note1wd4…hgt3# Introduction
LNbits is a project plagued with poor code quality, potential security risks and bad management. What can be done about it?
# The problems
## Improper handling of vulnerability disclosures
### Case 1
Two years ago (don't remember exactly when, Twitter account deleted), I discovered an SQL injection vulnerability in LNbits. This could be exploited very easily by setting by sending the injected data in keys in the payload of most requests. (anyone could create create wallets)
This was possible due to the query to insert/update was generated using the body of the request, with basically no sanitization.
When I contacted Ben Arc about this, **the fix for this exploit was not implemented for several months**, and when it was, I do not recall there being an advisory for people to update.
### Case 2
A month or two ago, I had discovered an exploit that allows draining nodes completely under certain conditions. I had reported this to an active maintainer of the project, which had passed it on to the team. **The exploit has not been fixed.**
### Case 3
Again, a month or two ago, a flaw was discovered in the SatsDice extension that allowed anyone to drain wallets. I had investigated this, and found that the extension allows invoice keys (instead of admin keys only) to create dice that have a guaranteed win rate and return more sats than put in, and then use it to drain balances.
I had reported this to the team, and while it was fixed *a week or two later*, there was no easily visible for people to update except an "update your SatsDice extension" message in the LNbits chat buried in a conversation about the exploit.
## Bad security practices
### "Don't do one thing, and don't do it well"
LNbits has over time expanded its scope, from being a wallet layer to an LN apps platform to a node management tool. While this may seem great for users, it has come at a significant security cost:
- There is a larger attack surface for attackers to exploit.
- The newly introduced node management and admin UI feature allows easy draining of nodes if the admin URL was leaked, whether by by the autocomplete on the search bar or the user's history.
- All extensions run at the same privilege level as LNbits itself which has direct access to funds.
### Developers can accidentally shoot themselves in the foot
The LNbits codebase encourages many harmful development patterns that allows developers to easily create vulnerabilities that put funds at risk
#### Example 1
Most LNbits extensions use the following system for handling object creation/deletion:
- Take the body and convert it to a class
- Overwrite/sanitize fields in that class, such as preventing overwrite of the wallet ID
- Pass it to the DB to write
There is a slight problem: it can be easy to miss what to sanitize and what to not sanitize.
When you are doing an update request, you may accidentally forget to overwrite the wallet_id on the request sent by the user to update a withdraw link, and trust the user. You just created a bug to drain anyone's wallet given its ID, which LNbits does not consider sensitive information and sometimes requires you to share.
#### Example 2
LNbits treats any error during a payment attempt an error. But errors don't always happen due to failures in your request. Network connectivity is flaky, nodes restart, and way more reasons for there to be an error while the payment actually may be going through.
If you can get a way to trigger an error during a payment call, but also have it succeed, you can easlly drain a node. I have reasons to believe this is possible in production deployments.
This could easily be fixed as attempting to check the status of a payment after an error, and if that also fails, consider the payment pending until it can be checked, but ths has not been implemented.
## Bad project management
LNbits' current management suffers from many problems:
- the project suffers from feature and scope creep, implementing things for the sake of it
- bugs go unaddressed
- security is not a top priority
This is problematic for a project that deals with funds, as it is everything that you shouldn't be doing for a project meant to deal with funds. Tens of thousands of dollars in some cases and thousands in others.
# What can we do about it?
For me, I have disabled withdrawals on my LNbits instance.
I am also working on a replacement project called LNLocker that will solely focus on the wallet layer bit.
I would encourage trying to replace LNbits, as hard and sometimes impossible as it may be, or if you can, modify the code or restricting access via LND macaroons so that withdrawals cannot be made.
I highly recommend not exposing LNbits to the internet if you cannot disable withdrawals.