On 2015-08-12 10:43:51 +0200, Andres Freund wrote: > 3) The 'bypassrls' boolean in AlterUser() is queried strangely. There are two > complaints: > > bool bypassrls = -1; > > it's a boolean. > > else if (authform->rolbypassrls || bypassrls >= 0) > { > if (!superuser()) > ereport(ERROR, > > (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), > errmsg("must be superuser to change bypassrls > attribute"))); > } > ... > if (bypassrls >= 0) > { > new_record[Anum_pg_authid_rolbypassrls - 1] = > BoolGetDatum(bypassrls > 0); > new_record_repl[Anum_pg_authid_rolbypassrls - 1] = true; > } > > if it's a boolean, that's always true. > > > It's not entirely clear to me why this was written that > way. Stephen?
Which incidentally leads to really scary results: postgres[25069][1]=# SELECT oid, rolname, rolbypassrls FROM pg_authid WHERE rolname = 'plain'; ┌───────┬─────────┬──────────────┐ │ oid │ rolname │ rolbypassrls │ ├───────┼─────────┼──────────────┤ │ 76892 │ plain │ f │ └───────┴─────────┴──────────────┘ (1 row) postgres[25069][1]=# ALTER ROLE plain NOLOGIN; ALTER ROLE postgres[25069][1]=# SELECT oid, rolname, rolbypassrls FROM pg_authid WHERE rolname = 'plain'; ┌───────┬─────────┬──────────────┐ │ oid │ rolname │ rolbypassrls │ ├───────┼─────────┼──────────────┤ │ 76892 │ plain │ t │ └───────┴─────────┴──────────────┘ (1 row) The -1 isn't a valid value for a bool, which is now unsigned, and just wraps around to true. There are no tests catching this, which is a scary in it's own right. So on linux/x86 this normally is only an issue when using a definition for bool other than c.h's (which we explicitly try to cater for!). But on other platforms the whole logic above is terminally broken: Consider what happens when char is unsigned. That's e.g. the case on nearly all, if not all, arm compilers. This is reproducible today in an unmodified postgres on x86 if you add -funsigned-char. Or, on any arm and possibly some other platforms. Whoa, allowing the compiler to warn us is a good thing. This would have been a fucktastically embarassing security issue. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers