datamapper / do

DataObjects
147 stars 73 forks source link

Fix use-after-free in do_postgres_raise_error #68

Closed jeremyevans closed 10 years ago

jeremyevans commented 10 years ago

The char* returned by PQresultErrorField is freed by PQclear (see http://www.postgresql.org/docs/9.3/static/libpq-exec.html), so this is a use-after-free. This causes a segfault on OpenBSD-current as free overwrites the memory with junk, both for security reasons and to detect programming errors like this.

I didn't actually test this (just edited the file on GitHub), but hopefully it works. If not, I'm sure you'll be able to come up with a similar fix.

jeremyevans commented 10 years ago

Looks like this isn't a complete fix. You are also using PQresultErrorMessage and referencing the char* after calling PQclear (inside data_objects_raise_error), which is a separate use-after-free, and harder to fix.

You could change the API of data_objects_raise_error to make the message argument a VALUE instead of a const char , but I'm guessing that is unacceptable as it will affect all drivers. Alternatively you could add a new method with a similar API to data_objects_raise_error but using a VALUE instead of a char , having data_objects_raise_error call that method. Then do_postgres_raise_error could call that new method

If you don't want to add/modify the API, the best way I can see to avoid the use-after-free while not leaking memory is to wrap the call to data_objects_raise_error inside do_postgres_raise_error with rb_ensure, and have the ensure function call PQclear.

postmodern commented 10 years ago

+1 use after frees are dangerous.

dbussink commented 10 years ago

Thanks! I'm going to take a look at the other issue and fix that after I get back from vacation. The failure seems to be due to missing packages on Travis, they were installed before. I've asked them about that issue.