Potential buffer overflow and sql injection bugs in oui-httpd @ session_login()

[retpoline]

Environment:
OpenWRT latest (untested)

Description:

oui/oui-httpd/src/session.c

Line 180 in 867faf3

sprintf(sql, "SELECT acl, password FROM account WHERE username = '%s'", username);

In session.c, session_login() calls sprintf() on a fixed-size buffer sql of size 256. A large username of > 256 bytes would overflow the sql buffer and possibly lead to remote code execution. Also, username is being concatenated into a SQL statement without any validation, so SQL code injection looks like be possible as well.

sprintf(sql, "SELECT acl, password FROM account WHERE username = '%s'", username);

...
db_query(...)

These also look to be triggerable non-authenticated, before login as this is in the login routine itself, but let me know if that's incorrect or there is actually auth somewhere else beforehand.

There is a length check on username and password in session_create(), but that's called after session_login(), so it's not helpful in preventing attacks here.

FIX: 1) ensure username and password are always smaller than eg. sizeof(SELECT ... statement), probably ok to put them both max size at 32 or 64, and 2) validate username is always alphanumeric only, plus non-injection characters if necessary, so SQLi isn't possible.

References:

• https://owasp.org/www-community/vulnerabilities/Buffer_Overflow
• https://owasp.org/www-community/attacks/SQL_Injection

[laoshaw]

Actually what's the goal for oui-http? it seems doing what uhttpd does, is this for performance reason? what's its advantage over uhttpd

[zhaojh329]

Actually what's the goal for oui-http? it seems doing what uhttpd does, is this for performance reason? what's its advantage over uhttpd

I can better control the progress of the project and facilitate the expansion of functionality

[retpoline]

@zhaojh329 hey Jianhui -- maybe I missed it, but did you get a chance to take a look at the issues described above?

[j123b567]

Better solution would be to never ever use s(n)printf to construct SQL statements by hand and always use sqlite3_bind functions instead.

[zhaojh329]

Vue3 + Nginx + Lua is coming...