Are minor "security fixes" welcome?

Hello!

The organization I’m working for runs Open edX through a number of security analysis tools as part of deploying it. There are a number of negligible-impact security issues that nevertheless set the scanners off.

For example: this SQL injection vulnerability probably isn’t a huge problem, as a user that can run this management command could just as easily run python manage.py lms dbshell and get direct database access, but SQL injection is considered a critical vulnerability, so it sets the scanners of and causes headaches for everyone.

I’m happy to submit patches that fix this sort of thing, but wanted to check that they’re not going to be more trouble than they’re worth for Open edX’s maintainers.

1 Like

If you find any vulnerabilities, it would be great if you could report them privately to security@edx.org – we have a process for reaching out to deployers ahead of publicly releasing a fix so that they have time to deploy the fix themselves. (Coordinated disclosure, essentially.) You can see edX’s responsible disclosure security policy here: https://www.edx.org/policy/security

The example you provide isn’t exploitable, even by an attacker with the ability to run a limited set of management commands, since the list of table names is hardcoded in the script and can’t be modified by command line arguments. But, it’s still a weakness, since theoretically the script could some day be modified to accept that dictionary as CLI input—and we should avoid having any code that constructs SQL from string concatenation. So this example is one where I’d personally be comfortable with you just submitting a PR, which could then be routed to the appropriate team for review. It’s not only non-exploitable in practice, but would require privileged access or a nonstandard configuration on top of that. (Vulnerabilities that require privileged access are sometimes still vulnerabilities. For example, if someone could run arbitrary code on the backend by accessing a Django server’s admin pages, that would be a problem and we’d want to know about it. Users of admin pages may only be given limited access to some models, after all.)

So I’d say use your discretion. If something looks like it’s just causing false positives due to poor coding practices, a PR would be great, but if you think it’s possible it could be exploited then an email to security@edx.org would be best. Either way, it’s appreciated!

Thanks for the quick and detailed reply!

Absolutely, all the instances the scanner found are non-exploitable. These submissions would be more about making Open edX more palatable for security teams who are overworked and under-familiar with Django and don’t have time to manually gauge the impact of everything their scans flag.

I would never drop a question like this if I thought it might have real security impact :sweat_smile:.

1 Like

Yeah, that sounds great! We’d very much appreciate it.