Unable to install gem; format-security error #9

Closed
jasonhutchens opened this Issue Mar 14, 2013 · 8 comments

Projects

None yet

2 participants

@jasonhutchens

I'm having an issue installing the sequel_pg v1.6.5 gem on a fairly vanilla AWS instance. It fails with this error:

sequel_pg.c:279:3: error: format not a string literal and no format arguments [-Werror=format-security]
@jeremyevans
Owner

The function is static, and all strings passed to it are static and do not have formatting characters in them, so what sequel_pg does is completely safe. If -Werror=format-security complains about it, that's an issue with that compiler option. I could switch sequel_pg to use "%s" and pass the string as an option, but working around broken compiler options is not something I want to do. Do you know why that option is being used? It's not something internal to sequel_pg.

@jasonhutchens

No, I'm not sure why that was being used; I haven't done much digging around. I reverted to v1.6.3 to fix the problem and logged the bug.

@jasonhutchens

I suppose the broken compiler flag could be suppressed in that one case by doing something like this: http://stackoverflow.com/questions/3378560/how-to-disable-gcc-warnings-for-a-few-lines-of-code

@jeremyevans
Owner

The thing is, this isn't a bug in sequel_pg. You are building sequel_pg with an compiler flag that raises an error instead of just a warning when it comes across a format string that it thinks may be vulnerable. However, I've explained that sequel_pg is not vulnerable (static function with all static strings without formatting characters as input).

Adding code to sequel_pg to work around broken compiler flags is not something I want to do, but if it will make things easier, I'm willing to apply the following patch:

diff --git a/ext/sequel_pg/sequel_pg.c b/ext/sequel_pg/sequel_pg.c
index 6646301..8dbfa59 100644
--- a/ext/sequel_pg/sequel_pg.c
+++ b/ext/sequel_pg/sequel_pg.c
@@ -276,7 +276,7 @@ static VALUE spg_timestamp_error(const char *s, VALUE self, const char *error_ms
       return rb_funcall(db, spg_id_infinite_timestamp_value, 1, rb_tainted_str_new2(s));
     }
   }
-  rb_raise(rb_eArgError, error_msg);
+  rb_raise(rb_eArgError, "%s", error_msg);
 }

 static VALUE spg_date(const char *s, VALUE self) {

Can you test that and see if it works for you?. If so, I'll apply it. I don't plan on putting out a gem release just for it, but it will be a part of the next release in that case.

@jasonhutchens

Yes, I understand that the bug isn't in sequel_pg. But the problem is that I'm not doing anything special; literally just trying to install the gem on an ubuntu box. I've since tried on some of our other servers, and 1.6.5 installs cleanly. Would it be better to close this as a known issue on some platforms?

@jeremyevans
Owner

Weird. I'm guessing the ubuntu config uses that as one of the default compiler options. I would say that's a bad choice, but that's just my opinion.

Since that patch isn't likely to break anything, works in my personal testing, and will probably fix cases like yours, I'll probably merge it fairly soon. Thanks for letting me know.

@jasonhutchens

No probs, in that case I'll test that it resolves my issue.

@jasonhutchens

I needed to do this after cloning the repo onto the problematic ubuntu box; the README.rdoc needs updating:

$ sudo gem install rake-compiler
Fetching: rake-compiler-0.8.3.gem (100%)
Successfully installed rake-compiler-0.8.3
1 gem installed

$ rake -f Rakefile.cross compile:sequel_pg
mkdir -p tmp/x86_64-linux/sequel_pg/1.9.3
cd tmp/x86_64-linux/sequel_pg/1.9.3
/usr/bin/ruby1.9.1 -I. ../../../../ext/sequel_pg/extconf.rb
checking for main() in -lpq... yes
checking for libpq-fe.h... yes
checking for PQsetSingleRowMode()... no
creating Makefile
cd -
cd tmp/x86_64-linux/sequel_pg/1.9.3
/usr/bin/ruby1.9.1 -I. ../../../../ext/sequel_pg/extconf.rb
checking for main() in -lpq... yes
checking for libpq-fe.h... yes
checking for PQsetSingleRowMode()... no
creating Makefile
cd -
cd tmp/x86_64-linux/sequel_pg/1.9.3
make
compiling ../../../../ext/sequel_pg/sequel_pg.c
../../../../ext/sequel_pg/sequel_pg.c: In function ‘spg_timestamp_error’:
../../../../ext/sequel_pg/sequel_pg.c:279:3: error: format not a string literal and no format arguments [-Werror=format-security]
cc1: some warnings being treated as errors
make: *** [sequel_pg.o] Error 1
rake aborted!
Command failed with status (2): [make...]
/var/lib/gems/1.9.1/gems/rake-compiler-0.8.3/lib/rake/extensiontask.rb:112:in `block (2 levels) in define_compile_tasks'
/var/lib/gems/1.9.1/gems/rake-compiler-0.8.3/lib/rake/extensiontask.rb:111:in `block in define_compile_tasks'
Tasks: TOP => compile:sequel_pg => compile:sequel_pg:x86_64-linux => copy:sequel_pg:x86_64-linux:1.9.3 => tmp/x86_64-linux/sequel_pg/1.9.3/sequel_pg.so
(See full trace by running task with --trace)

$ git apply issue9.patch

$ rake -f Rakefile.cross clean
rm -r rdoc
rm -r tmp/x86_64-linux/sequel_pg/1.9.3
rm -r tmp/x86_64-linux/sequel_pg/1.9.3

$ rake -f Rakefile.cross compile:sequel_pg
mkdir -p tmp/x86_64-linux/sequel_pg/1.9.3
cd tmp/x86_64-linux/sequel_pg/1.9.3
/usr/bin/ruby1.9.1 -I. ../../../../ext/sequel_pg/extconf.rb
checking for main() in -lpq... yes
checking for libpq-fe.h... yes
checking for PQsetSingleRowMode()... no
creating Makefile
cd -
cd tmp/x86_64-linux/sequel_pg/1.9.3
/usr/bin/ruby1.9.1 -I. ../../../../ext/sequel_pg/extconf.rb
checking for main() in -lpq... yes
checking for libpq-fe.h... yes
checking for PQsetSingleRowMode()... no
creating Makefile
cd -
cd tmp/x86_64-linux/sequel_pg/1.9.3
make
compiling ../../../../ext/sequel_pg/sequel_pg.c
linking shared-object sequel_pg.so
cd -
cd tmp/x86_64-linux/sequel_pg/1.9.3
make
make: Nothing to be done for `all'.
cd -
install -c tmp/x86_64-linux/sequel_pg/1.9.3/sequel_pg.so lib/sequel_pg.so
install -c tmp/x86_64-linux/sequel_pg/1.9.3/sequel_pg.so lib/sequel_pg.so

So all works as expected. Thanks for your quick response time btw ;)

@jeremyevans jeremyevans added a commit that closed this issue Mar 15, 2013
@jeremyevans Work around format-security false positive (Fixes #9)
Even though the rb_raise call is provably safe (static function
called with all static arguments), format-security still complains.
To appease it and make sequel_pg work in setups where
-Werror=format-security is used by default, use "%s" as the format
string.
cd097a6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment