WordPress security plugin Hide My WP addresses SQL injection, deactivation flaws | Hacker News Reader

WordPress security plugin Hide My WP addresses SQL injection, deactivation flaws

portswigger.net

75 pointspatchstack51 comments

SQL injection draws attention to the fact that almost all web applications run as root, in regards to the most important asset: the data.

It’s odd that this hasn’t got more attention. It should be easier to write backends that tie data access more closely to user credentials without the backend trying to enforce that itself. Is there anything out there that makes this easy to do?

> almost all web applications run as root

That‘s largely a configuration problem. They don‘t have to run as root, most of the time.

With regards to the database. Not UNIX root.

Should there ever be a web application that needs to run as root, it should be rewritten. All logic requiring root privileges should be moved into a separate system service with a minimal attack surface to which the web application delegates those special tasks.

You misunderstood the grandparent's comment and replied to a confused parent. The application does not need to run as root, however even when it runs as a normal user, it generally has full read/write access to the application database. Which is a problem that is not easily solvable.

I think you need root permissions to start a network service on port 80 on Windows. There is a security risk here which was addressed in a fairly recent Chrome release that local addresses need a respective CORS rule. At least I believe so.

Disadvantage with the Windows behavior is that a webservice running on said port has elevated permissions and if you surf the web with the same machine, a script inserted into a site could launch attacks against it. Of course the site itself could be malicious, in that case the CORS rule wouldn't do much.

Of course you are correct, running directly on port 80 might just be a bad idea, but for development purposes I might do that.

Ports < 1024 are defined as "privileged", so it's not just a Windows thing.

What usually happens is that the process only has the necessary capability for a brief period to open the port and drops it immediately before processing any requests.

It was poorly phrased, but the OP meant that web apps usually have full unrestricted access to the database. Clearly that's not entirely true (your webapp likely doesn't need truncate, or the ability to add/change stored procedures, or the schema) but it's broadly there. Your database is effectively chmod o+r

There is row level security. But that usually means many db users or sometimes usergroups are supported.

https://www.postgresql.org/docs/current/ddl-rowsecurity.html

There isn't, at least not really, since webapps are designed as full-service kiosks: all of the data management, user management and access control are supposed to be handled from the same console. If you want the database to enforce access controls, then every user must exist as a separate entity in the database, which is good, but then the webapp also needs the permissions to manage user accounts in the database, which is bad for security, and the webapp can no longer use connection pooling, which is bad for performance.

There are tricks you can use, such as implementing row-level security and tying it to some concept of end user identity, but even ignoring that ORM's don't understand or support RLS and developers therefore won't use it: it's still the same problem of letting the fox guard the henhouse. The webapp is the single control point for both user access and user administration (including self-service account creation and password reset), so whatever solution you come up with, the webapp will need to have super-user access to do that, and therefore if the webapp is compromised, your data-level access controls can be compromised too.

So, in order to even begin thinking about securing your data, the webapp should probably be split into multiple reduced-access microservices that handle different aspects of the webapp function. As long as we're talking about a single monolithic backend, any attempt at scope mitigation can (and likely will) be defeated.

It's not that we don't know how to build things securely, it's just that we routinely ignore even basic principles of security 'cause it's easier. Virtually every webapp violates complete mediation and so it's just unreasonable to expect security. Similarly, most developers don't care for security and even less about principles and system design. Again, it's unreasonable to expect secure outcomes from that.

It's probably more charitable to say "most devs are not given time and effort bandwidth to care about security". Trust me I'd love to plug more holes before first production deployment but management (and my CTO) would never approve a time/energy budget for it.

In the current market, I hope it won't sound too disingenuous to suggest you look to work with management that does. You'll be happier, and the market signal of enough people doing it will improve conditions for everyone.

I don't disagree in principle but there are many other factors that increase my job happiness:

- We have deadlines but they are not strict and very rarely somebody is going to even ping you outside work hours. People there want some discipline but they don't micromanage.

- The atmosphere is 95% of the time super chill.

- There are no egos flying around (not more than usual anyway; I scheduled a meeting to explain some coding style practices that should be painfully obvious but hey, trying not to judge here).

- Technical excellence is appreciated by a very technical and careful CTO -- but is indeed often sidelined in favor of deadlines and business-enabling work. However, I have already successfully fought him and the CEO off on 2-3 technical excellence tasks by demonstrating they'll reduce future slowdowns. Doable but requires some brawling in meetings.

- Pay is one of the biggest in EU (although it's like 1.5x - 2.0x less the than the US one).

---

> and the market signal of enough people doing it will improve conditions for everyone.

I keep hearing this and I want it to be true but for 20 years of career I've never seen it, not once. Nowadays I no longer believe it. Everywhere I worked (I am mostly a contractor so it has been a very colorful career) the business will stick to their idea of "we can always hire somebody else" with persistence that you'd be jealous of -- I don't know if it's an illusion or not but trust me, it's VERY persistent. That alone weakens the point you brought up because employers simply don't believe it and often times go out of their way to look for those other mythical people -- and I've been told in several occasions that the business closed doors before they managed to repair their clusterf_ck of an app. That's how persistent they are in their thinking process that everyone is expendable.

There are some easy wins most applications could use but don‘t most of the time. Like having sensible permissions for your app-db-user. Running migrations in a separate process? No need to have permission to do schema changes. Events are only written to the db and not queried/viewed from the app itself? Grant only INSERT rights to the table etc.

I have experimented with a design that only allows the application to access a set of stored procedures via it's SQL user.

It seems to work, if you write complicated enough stored procedures you can enforce things like each entry must be created by a logged in user or whatever. Then the application is limited to interactions it's supposed to have, so it can't drop a audit table or something. I even did login via a stored procedure, confirming the hashed password matched and generating a bearer token that future stored procedures require to function.

My current project is maintenance for a large system built of shared procedures.

In principle this feels like a reasonable strategy but chances are your stored procedures do not enjoy the same creature comforts as your "real" software. In my case they've got commented out code blocks, procedures with names ending V3.bak.tmpMar14.old, randomly different styles and formats from one to the next, and inconsistent naming (AllGroups allGroups or ag?). All of which would be quickly and easily prevented or retrospectively corrected in say C# or Java but there's no easy way to do it with the archaic T/SQL setup here.

I agree it would require a great deal more discipline to implement then the alternatives. I can do it by myself, just takes about twice as long to do over the normal way, but with a team it would be a nightmare.

I'm unfamiliar with T/SQL but of those issues seem like fundamental limitations of stored procedures, just implementation details that need updating. Why is there no way to do it?

Of course, everything is possible, but resources are limited and so it matters that it's harder and requires ongoing effort to keep this right.

Edited to add: I retrospectively realised the above comment had an auto-correct mistake where "easy way" had been misscorrected to "way way" and so it wasn't clear I meant only that it's needlessly harder

Even in a monolith, scope can be managed. I've worked on an application where a specific subset of endpoints used a different data source that was configured with different credentials, attached to a read-only user. Without code execution, these systems were reasonably fool-proof.

A plugin system where an plugin would be exposed to a different set of credentials shouldn't be too hard to set up with some middleware preparations. Such a system would require granting the main account complete database control (which is iffy) or would require a lot of manual configuration for user permissions (which sucks) but it's definitely something you can do in a monolith like WordPress.

Permissions wouldn't be as well-contained as in a proper, fragmented, micro-service-oriented permissions model, but it would be a good step forward and one that wouldn't necessarily break too much when done as a software update.

If your database supports session wide environment variables you can attach user/group/permissions object to the session and have your queries/functions/procedures check that.

If your database supports writable views (e.g. via triggers) you can build parametric views.

Both are not plug-and-play modules in your framework and requires actual DBA, though.

I always wondered, how do they exfiltrate the data? The website isn’t designed to display any schema. Do they save it as file from the SQL server? Can’t that be locked down? The attacker would still be able to delete but at least not extract.

If you're able to inject SQL, and the return value of the SQL query is not directly displayed to the user, you may use a timing-based side channel to exfiltrate data.

E.g. in order to exfiltrate the string "Test123" you would go character-by-character, starting with the first character "T". For each ASCII character you would wait 10ms, as "T" is ASCII #84 [1] you'd sleep() for 84*10=840ms. This sleep() can be measured from the attacker side because the SQL query will block the HTTP response.

This way, without "seeing" a result, the attacker is able to return data.

[1] https://en.m.wikipedia.org/wiki/File:ASCII-Table-wide.svg

We're there any actual timings attacks executed like this? I'm talking about stack "attacker => internet => reverse-proxy => app => database", not extracting data over LAN from exposed app.

Yes, that's one way to extract data by (ab)using a blind sql injection vulnerability.

If you look at sqlmap [1], they offer two techniques for blind sql injection: boolean-based and time-based. Boolean-based should be used when the app just returns an error page (or not) based on your sql injection. The time-based approach should be used when no error page appears but the SQL is still executed.

But when I look at sqlmap docs for the time-based approach [2] I think I got the initial explanation wrong. It will do a 5 second delay if a certain condition is met, e.g. "Is the first character of the value an 'T'? If yes, wait 5 seconds; if not, return immediately". And then send hundreds of requests in parallel to iterate over all positions & possible characters.

[1] https://github.com/sqlmapproject/sqlmap/wiki/Usage#sql-injec... [2] https://github.com/sqlmapproject/sqlmap/wiki/Usage#seconds-t...

SQL injections for me are solved problem.

Frameworks and ORMs are making it easier on back-ends to tie data access to user credentials. The same with multi-tenancy if you use good framework with ORM you have all the tools to do filtering on higher level of abstraction. The same with referential consistency on database if it is there - ORM will help you to load data that is tied to that account. Making user like web app users on database level would make all development/ops really costly.

If someone is writing SQL queries directly, he has to have a good reason for it. Like if it is plugin for Wordpress it probably is not that easy to use ORM.

This plugin has a low install count for WordPress at around 26K. It’s a commercial plugin as opposed to one of the plugins in the WP repo which helps explain the low install count.

Most of the plugins that WP researchers look at are in the official repo with higher install counts, so it’s nice to see stuff like this get some code coverage among researchers.

Envato should really invest in their own automated and human vulnerability research. I think there’s probably a lot of badness out there among the commercial plugins but in many cases researchers have to buy the plugin to take a look.

WP can't fix this entire class of vulnerabilities properly or it won't be Wordpress any more. It is unfortunate that Wordpress is one of the worse options for making websites today (not WP itself, but any theme or plugin can easily be coded to have SQLi), yet it is the most popular.

This is one of those types of things that shouldn’t happen anymore, but does.

I’m pretty sure that WordPress now has their own low-level version of PDO Prepared Statements. Also, they have a lot of even higher-level DB abstractions. I can’t think of any reason to directly access the DB from a plugin or theme.

> I can’t think of any reason to directly access the DB from a plugin or theme.

Because it's PHP and it's easy.

Too true. It’s not PHP’s fault, though. The same type of argument is applied to C++.

We keep developing “nerf-world” languages, designed to protect ourselves from ourselves, and most fall down before they get a chance to even get going; mostly because they constrain, without empowering. I remember moving to Pascal, after using Assembly and Machine Code. It was very frustrating for me. Pascal was one of the earliest “safe” languages.

The problem is that we can’t build houses, using PlaySkool “Li’l Builder” toolsets.

Also, the WP Codex is really disorganized. It’s difficult to find anything in it, and that is deadly.

Geeks like to design clever architectures, but we hate to document them.

>I’m pretty sure that WordPress now has their own low-level version of PDO Prepared Statements.

I imagine it's well tested, but it's a bunch of PHP escaping and regexes inside of wp-db.php. It is not at all real placeholders and prepared statements, though the functions are named that way. I suppose because there's too much tech debt to use normal placeholders.

Ah ... well, it's better than nothing.

For me, I always use PDO, and prepared statements and transactions. I get a lot of power, for free. I'm not a particularly good DB programmer, so I need all the help I can get.

Not surprising to me. I work at a small company, focused on industrial automation. It can be very expensive and time-consuming to automate things, so our internal processes are ironically manual. Customers don't pay us to automate our internal processes, so that never gets done.

But it doesn’t cost more to use a parametrised query. That’s dev incompetence not cost pressure.

Here are full details of that vulnerability: https://patchstack.com/hide-my-wp-vulnerabilities-fixed/

I'm not familiar with WP under the hood. Am I being naive or is it feasible to write a routine that scans plug-ins source code looking for insecure query argument handling?

semgrep has php support, so it should be possible, at least with respect to the “basic” APIs. If a plugin wraps the raw sql apis I don’t think semgrep can “inline” the information, so you’d have to reach for the taint tracking but that’s way more complicated than just check rules.

There’s a couple of excellent static analysis tools (phpstan, psalm) that can catch this kind of stuff. I’m 99% sure that psalm’s security suite would catch this.

Weird - or, oddly, not surprising - that no one seems to be using these tools

Newrelic synthetic check cannot be used to verify that newrelic site is functional(their synthetic checks use Chrome 72, their site understandably doesn't support such old browsers)

There's a saying here that the shoemaker's children walk barefoot. I think this might not be an isolated phenomenon

Ironic

This is ridiculous. SQLi is a dumb problem and any development environment worth using should make it trivial to avoid. All you have to do is never allow SQL queries to be formed using string concatenation. Or if you do, make sure you're excruciatingly strict about it.

I run a large-ish web application written in ColdFusion (more precisely, CFML running on the open source Lucee) which is a language where string concatenation is happening all the time. This is a very old language which will look embarrassingly janky compared to any of the new hotness in vogue on Hacker News. But even in CFML I can trivially guarantee zero SQLi by parameterising all variables.

Query parameters. They're a thing. If you don't know how your favourite programming language has implemented them, learn it. Use them.

I agree of course, and in the case of the article you're absolutely right, although more generally there are edge cases where parameterization doesn't work, like giving choice of which row to order by. Naturally in such cases you should be operating on a whitelist of options and not pass the user input to the query, but the point is that some situations do still have to be accounted for.

I have no problem placing "if" or "switch" statements within the SQL in order to enable/disable various components of the query. The risk is when you're bringing in anything other than local constants.

In the rare instances where I do this, I'm analysing the full scope of potential valid variables. I then make local copies of the variables which have been pummelled with the narrowest possible regular expression, e.g.

  slightly_safer = unsafe.replace(/[^0-9A-Za-z]/gi, '')

Stripping away all punctuation eliminates most forms of SQLi.

for mysql at least, if you're dealing with table names you can just do a backtick escape, where you wrap the table/column name in backticks and escape any backticks within it with two backticks. That's usually my go-to in situations like this, in addition to a whitelist. anything else you can usually parameterize

Relying on escape sequences is a serious footgun risk as there's always going to be some weird scenario that can be exploited against you, such as alternative ways to escape backticks that quietly gets enabled when some unexpected incident forces you to rebuild your MySQL Server instance.

When you're bringing in strings from outside, there's no such thing as being too paranoid. White-listing of permitted characters is the only safe approach. (And if you think your whitelist might ever need contain any kind of quote-mark, backtick or backslash, then you're wrong. Your table/identifier names are stupid and must change.)

In WordPress the string concatenation in SQL is normal, even on the very first example:

    $results = $wpdb->get_results( "SELECT * FROM {$wpdb->prefix}options WHERE option_id = 1", OBJECT );

This is because the database tables aren't static, so one must add prefix to each table from a variable. It's horrible, it would have been far better if there was magic string like %PREFIX% to avoid that.

[1]: https://developer.wordpress.org/reference/classes/wpdb/#usin...

Or even better if they didn't treat tables like confetti. I haven't analysed the wordpress schema closely but on first blush it looks like a hacky premature optimisation to solve a problem, either one of performance, or data isolation, or to avoid unravelling use of magic numbers.

Realistically, content within the same "universe" should co-mingle in the same table. Multiple "universes" of content should be isolated into their own database/schema, which $wpdb should pre-select upon connection.

  "SELECT * FROM options WHERE prefix = ? AND option_id = 241"

> “The function hmwp_get_user_ip tries to retrieve the IP address from multiple headers, including IP address headers which can be spoofed by the user such as X-Forwarded-For,” reads a blog post published by Jong yesterday (November 24). ”By supplying a malicious payload in one of these IP address headers, it will be directly inserted into the SQL query which makes SQL injection possible.”

> wpWave [adressed] both flaws in Hide My WP version 6.2.4, released on October 26.

Reply on news.ycombinator.com