View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001273 | Main CAcert Website | source code | public | 2014-04-19 01:39 | 2014-12-05 00:45 |
| Reporter | NEOatNHNG | Assigned To | NEOatNHNG | ||
| Priority | high | Severity | minor | Reproducibility | N/A |
| Status | closed | Resolution | fixed | ||
| Platform | Default | OS | any | OS Version | any |
| Product Version | 2014 Q2 | ||||
| Target Version | 2014 Q2 | Fixed in Version | 2014 Q4 | ||
| Summary | 0001273: Replace all backtick operators with calls to runCommand() or shell_exec() | ||||
| Description | Backtick operators are very hard to search for and should therefore be avoided (especially because the backtick character also serves as identifier escaping in MySQL). Replace all backtick operators with calls to the appropriate function and fix any missing shell argument escaping along the way. | ||||
| Tags | No tags attached. | ||||
| Reviewed by | NEOatNHNG, BenBE | ||||
| Test Instructions | Verify that all processes mentioned in #c4854 are still working. For test instruction see #c5022 | ||||
|
|
A fix is available here: https://github.com/yellowant/cacert-devel/commits/bug-1273 I locacted all places in a semi-automated manner (see commit message) |
|
|
I updated the patch so that former 'echo "$user_input"|command' will now be executed with "runCommand" |
|
|
The following processes are affected by this change: build a client key with browser gen (includes/account.php:405) build a client key with csr (includes/account.php:469) check if the whois-email is possible to choose for domain check (includes/account.php:573) generate a server cert (includes/account.php:744,) renew it ( includes/account.php:915, includes/account.php:945) build a client org key with browser gen (includes/account.php:1562) build a client org key with csr (includes/account.php:1616) create org server certificate (includes/account.php:1870) and renew it change password to "american-english" word (includes/general.php:222) add an additional email address(includes/general.php:543) sign gpg key (www/gpg.php) |
|
|
Second review OK. |
|
|
build a client key with browser gen => OK build a client key with csr => OK check if the whois-email is possible to choose for domain check ( fail... but error is not directly related to this bug pushed a fix: https://github.com/yellowant/cacert-devel/commit/d6e99bdb72f1d616a1a7e08c333f7c2079edaaf9) generate a server cert => OK renew it => OK build a client org key with browser gen =>OK build a client org key with csr/IE generation => (couldnt test) create org server certificate => OK renew it => OK change password to "american-english" (tested "ashtray" showed "0", without dict would be "1") => OK add an additional email address => OK sign gpg key => OK ==> nearly OK (couldn't test "whois") |
|
|
I do not get how to test this. I can do all this stuff, (and mostly have done this a lot, recently) but I will not be able to tell the difference. Please tell me how to say if a test is ok or failed and what to look out for. It is hard to tell from the outside, what calls were done on the inside. |
|
|
The non-existence of any difference is exactly what makes the difference to make this test successful. |
|
|
At least the escaping could make a difference, if some was missing, before. It is hard to test for this anywhere. Also this test probably should require to monitor the logfiles. |
|
|
Can the Test-Instructions please be verified by a Software Assessor? It is quite special that ONLY american stuff should be affected, for exaple. |
|
|
Test description: Account stuff: - try to add a password which is listed in an American-English dictionary - add a new email address - add a new domain (check for mail from whois is not implemented on test server) Cert Stuff: - create new client cert with browser - create new client cert with csr - create new server cert - renew server cert - create org client cert - create org server cert - renew org server cert - sign gpg key Result should be that all tests are successful |
|
|
Account stuff: - try to add a password which is listed in an American-English dictionary. I was able to add the following password "Elevator 1" => fail - add a new email address => ok - add a new domain =>ok Cert Stuff: - create new client cert with browser => ok - create new client cert with csr => ok - create new server cert => ok - renew server cert => ok - create org client cert => ok - create org server cert => ok - renew org server cert => ok - sign gpg key => ok Password failed, rest ok |
|
|
I fetched this evening at 20:20 h from git git clone git://git-cacert.it-sls.de/cacert.git production git clone git://git-cacert.it-sls.de/cacert-devel.git test-server and run these commands find production -type f -name \*php -print -exec egrep -n -e '\$\w+\s*=\s*`' {} \; >> /tmp/production.txt find test-server -type f -name \*php -print -exec egrep -n -e '\$\w+\s*=\s*`' {} \; >> /tmp/test-server.txt result for test-server is test-server/scripts/gpgfillmissingkeyid.php 42: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; test-server/scripts/gpgcheck3.php 113: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; test-server/scripts/gpgfillmissingemail.php 57: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; test-server/scripts/cron/warning.php 42: $subject = `openssl x509 -in $crt_name -text -noout|grep Subject:`; test-server/www/gpg.php 528: $do=`gpg --homedir $cwd --batch --export-options export-minimal --export $cmd_keyid >$csrname`; test-server/www/api/ccsr.php 78: $do = `/usr/bin/openssl req -in $incsr_esc -out $checkedcsr_esc`; 100: $do = `../../scripts/runclient`; test-server/includes/account.php 405: $res=`openssl spkac -verify -in $CSRname_esc`; 469: $do = `/usr/bin/openssl req -in $tmpfname_esc -out $tmpname_esc`; // -subj "$csr"`; 945: $cert = `/usr/bin/openssl x509 -in $crt_name`; 1562: $res=`openssl spkac -verify -in $CSRname_esc`; 1616: $do = `/usr/bin/openssl req -in $tmpfname_esc -out $tmpname_esc`; 2094: $cert = `/usr/bin/openssl x509 -in $crtname`; test-server/includes/general.php 222: $do = `grep -F -- $shellpwd /usr/share/dict/american-english`; 530: $do = `/usr/bin/gpg --homedir /home/gpg --clearsign "$tmpfname"|/usr/sbin/sendmail "$to"`; test-server/pages/account/15.php 33: $cert = `/usr/bin/openssl x509 -in $crtname`; test-server/pages/account/19.php 34: $cert = `/usr/bin/openssl x509 -in $crtname`; test-server/pages/account/6.php 63: $cert = `/usr/bin/openssl x509 -in $crtname $outform`; 85: $cert = `/usr/bin/openssl x509 -in $crtname -outform DER`; 114: $cert = `/usr/bin/openssl x509 -in $crtname -outform PEM`; test-server/pages/account/23.php 33: $cert = `/usr/bin/openssl x509 -in $crtname`; result for production is production/scripts/gpgfillmissingkeyid.php 42: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; production/scripts/gpgcheck3.php 113: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; production/scripts/gpgfillmissingemail.php 57: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; production/scripts/cron/warning.php 42: $subject = `openssl x509 -in $crt_name -text -noout|grep Subject:`; production/www/gpg.php 522: $do=`gpg --homedir $cwd --batch --export-options export-minimal --export $cmd_keyid >$csrname`; production/www/api/ccsr.php 78: $do = `/usr/bin/openssl req -in $incsr_esc -out $checkedcsr_esc`; 100: $do = `../../scripts/runclient`; production/includes/account.php 399: $res=`openssl spkac -verify -in $CSRname_esc`; 463: $do = `/usr/bin/openssl req -in $tmpfname_esc -out $tmpname_esc`; // -subj "$csr"`; 938: $cert = `/usr/bin/openssl x509 -in $crt_name`; 1561: $res=`openssl spkac -verify -in $CSRname_esc`; 1615: $do = `/usr/bin/openssl req -in $tmpfname_esc -out $tmpname_esc`; 2092: $cert = `/usr/bin/openssl x509 -in $crtname`; production/includes/general.php 222: $do = `grep -F -- $shellpwd /usr/share/dict/american-english`; 530: $do = `/usr/bin/gpg --homedir /home/gpg --clearsign "$tmpfname"|/usr/sbin/sendmail "$to"`; production/pages/account/15.php 33: $cert = `/usr/bin/openssl x509 -in $crtname`; production/pages/account/19.php 34: $cert = `/usr/bin/openssl x509 -in $crtname`; production/pages/account/6.php 63: $cert = `/usr/bin/openssl x509 -in $crtname $outform`; 85: $cert = `/usr/bin/openssl x509 -in $crtname -outform DER`; 114: $cert = `/usr/bin/openssl x509 -in $crtname -outform PEM`; production/pages/account/23.php 33: $cert = `/usr/bin/openssl x509 -in $crtname`; please check and correct after correcting and fetching the patched correct sources git checkout origin/testserver-stable the result is nochmal - das bleibt jetzt übrig bug-1273/scripts/gpgfillmissingkeyid.php 42: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; bug-1273/scripts/gpgcheck3.php 113: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; bug-1273/scripts/gpgfillmissingemail.php 57: $gpg = `gpg --with-colons --homedir /tmp $crt 2>/dev/null`; bug-1273/scripts/cron/warning.php 44: $subject = `openssl x509 -in $crt_name -text -noout|grep Subject:`; |
|
|
I updated the patches on github to include the patch for "scripts/cron/warning.php" |
|
|
1. for (0005023) "Elevator 1" => fail. Was clarified: "Elevator" was actually missing in the dictionary, while "elevator" was contained and the code always did not ignore the case. "elevator" was rejected as password. 2. magu and INOPIAE successfully tested the additional patch for the warning. We triggered such a warning and both received the correct email. (Additionally it was checked, that the special case, when information was missing in the database, was also correctly handled) |
|
|
https://bugs.cacert.org/view.php?id=1273#c5078 this is correct |
|
|
Account stuff: - try to add a password which is listed in an American-English dictionary: I tried to change the pw to 'membership' and it wasn't accepted, but the error displayed does not indicate clearly, that the rejection is actually caused by a matching word in the dictionary. => OK - add a new email address: Can add a new email address => OK - add a new domain: Worked as expected. => OK (check for mail from whois is not implemented on test server) Cert Stuff: - create new client cert with browser: Correctly generated and listed. => OK - create new client cert with csr: Correctly created and listed. => OK - create new server cert: Correctly created and listed. => OK - renew server cert: Correctly renewed. => OK - create org client cert: *tested with browser => OK *tested with csr => OK - create org server cert: Correctly created and listed. => OK - renew org server cert: Correctly renewed. => OK - sign gpg key: Correctly created and listed. => OK |
|
|
Sent to crit for deployment. |
|
|
The fix has been installed on the production server on November 24, 2014. See also: https://lists.cacert.org/wws/arc/cacert-systemlog/2014-11/msg00016.html |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-04-19 01:39 | NEOatNHNG | New Issue | |
| 2014-04-19 01:39 | NEOatNHNG | Assigned To | => NEOatNHNG |
| 2014-04-26 11:52 | BenBE | Status | new => needs work |
| 2014-04-26 11:52 | BenBE | Category | => source code |
| 2014-06-15 09:04 | felixd | Note Added: 0004832 | |
| 2014-06-15 09:52 | felixd | Note Added: 0004834 | |
| 2014-06-15 09:53 | felixd | Note Edited: 0004834 | |
| 2014-06-15 16:25 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 884770fd |
| 2014-06-15 16:25 | felixd | Source_changeset_attached | => cacert-devel testserver-stable e288e77f |
| 2014-06-15 16:25 | felixd | Source_changeset_attached | => cacert-devel testserver-stable fdd9deca |
| 2014-06-15 16:25 | felixd | Source_changeset_attached | => cacert-devel testserver-stable b6ee5404 |
| 2014-06-15 16:32 | BenBE | Reviewed by | => BenBE |
| 2014-06-15 16:32 | BenBE | Status | needs work => needs review & testing |
| 2014-06-17 22:04 | felixd | Note Added: 0004854 | |
| 2014-06-22 00:47 | NEOatNHNG | Reviewed by | BenBE => NEOatNHNG, BenBE |
| 2014-06-22 00:47 | NEOatNHNG | Note Added: 0004867 | |
| 2014-06-22 00:47 | NEOatNHNG | Status | needs review & testing => needs testing |
| 2014-07-01 23:59 | felixd | Note Added: 0004878 | |
| 2014-07-08 20:55 | Eva | Note Added: 0004880 | |
| 2014-08-07 20:54 | BenBE | Note Added: 0004921 | |
| 2014-08-08 06:07 | Eva | Note Added: 0004922 | |
| 2014-09-23 19:43 | felixd | Test Instructions | => Verify that all processes mentioned in #c4854 are still working. |
| 2014-09-23 19:53 | Eva | Note Added: 0005021 | |
| 2014-09-23 19:55 | Eva | Note Edited: 0005021 | |
| 2014-09-23 20:01 | INOPIAE | Note Added: 0005022 | |
| 2014-09-23 20:02 | INOPIAE | Test Instructions | Verify that all processes mentioned in #c4854 are still working. => Verify that all processes mentioned in #c4854 are still working. for instruction see #c5022 |
| 2014-09-23 20:03 | INOPIAE | Test Instructions | Verify that all processes mentioned in #c4854 are still working. for instruction see #c5022 => Verify that all processes mentioned in #c4854 are still working. For test instruction see #c5022 |
| 2014-09-23 20:16 | INOPIAE | Note Edited: 0005022 | |
| 2014-09-23 20:36 | INOPIAE | Note Added: 0005023 | |
| 2014-09-23 20:36 | INOPIAE | Note Edited: 0005022 | |
| 2014-10-21 18:52 | reinhardm | Note Added: 0005063 | |
| 2014-10-21 18:56 | reinhardm | Note Edited: 0005063 | |
| 2014-10-21 19:45 | reinhardm | Note Edited: 0005063 | |
| 2014-10-21 20:40 | felixd | Note Added: 0005067 | |
| 2014-10-21 20:50 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 350a2f0d |
| 2014-10-21 20:50 | felixd | Source_changeset_attached | => cacert-devel testserver-stable c2cc1c5e |
| 2014-10-28 22:16 | felixd | Note Added: 0005078 | |
| 2014-10-28 22:30 | MartinGummi | Note Added: 0005080 | |
| 2014-10-28 22:31 | MartinGummi | Note Edited: 0005080 | |
| 2014-10-28 23:40 | janmaco | Note Added: 0005081 | |
| 2014-10-28 23:42 | felixd | Status | needs testing => ready to deploy |
| 2014-11-23 15:06 | BenBE | Note Added: 0005121 | |
| 2014-11-23 16:00 | BenBE | Source_changeset_attached | => cacert-devel release 0bae20ba |
| 2014-11-24 10:18 | wytze | Note Added: 0005124 | |
| 2014-11-24 10:18 | wytze | Status | ready to deploy => solved? |
| 2014-11-24 10:18 | wytze | Fixed in Version | => 2014 Q4 |
| 2014-11-24 10:18 | wytze | Resolution | open => fixed |
| 2014-12-05 00:45 | NEOatNHNG | Status | solved? => closed |
