View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001199 | Main CAcert Website | GPG/PGP | public | 2013-08-03 15:39 | 2014-02-22 07:20 |
| Reporter | ansgar | Assigned To | BenBE | ||
| Priority | high | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | 2013 Q3 | ||||
| Fixed in Version | 2013 Q4 | ||||
| Summary | 0001199: arbitrary code injection | ||||
| Description | www/gpg.php contains: ---- $do = `echo "$debugkey\n--\n$debugpg\n--" >> /www/tmp/gpg.debug`; ---- This allows to inject arbitrary code if one controls the content of $debugpg which is quite easily possible. I was able to exploit this in the test image offered on [1], more details available on request. [1] <http://wiki.cacert.org/SystemAdministration/Systems/Development> For added fun this doesn't seem to be logged anywhere and looks like a legitimate request. Using `...` should really be avoided (so should mysql_real_escape_string, parameter binding exists since ages and looks much more readable). Ansgar | ||||
| Tags | No tags attached. | ||||
| Reviewed by | NEOatNHNG, BenBE | ||||
| Test Instructions | |||||
|
|
I have removed the debug code which should fix this problem. Please test & review. |
|
|
Patch is okay. |
|
|
Tests documented on 0001200. Mail sent to critical admins. |
|
|
The patch has been installed on the production server on August 29, 2013. See also: https://lists.cacert.org/wws/arc/cacert-systemlog/2013-08/msg00005.html |
|
|
The same problem still happens in other places in the same file: ---- $err = runCommand("gpg --with-colons --homedir $tmpdir 2>&1", clean_gpgcsr($CSR), $gpg); ---- Note that the contents of $gpg is still fully controlled by the user (gpg doesn't always do what you expect, try --store). ---- foreach(explode("\n", $gpg) as $line) { $bits = explode(":", $line); [...] $keyid = $bits[4]; ---- So one can also control $keyid. Later one can reach ---- $gpg = trim(`gpg --homedir $cwd --with-colons --fixed-list-mode --list-keys $keyid 2>&1`); ---- and the shell will do the rest (thanks to `...`). Ansgar |
|
|
Do you have a proof of concept key or instructions on how to construct one? I tried to follow your description but the key ID variable is extracted from GnuPG and no prior column allows for colons. I haven't checked yet if you could insert a newline to force splitting the UID entry, but given that the current implementation only likes one Public Key for Signing would not get that key signed due to supposedly multiple keys. Nonetheless that's pretty aweful XSS there that should be fixed. Also trusting the Key ID without further filtering should be changed. |
|
|
cat <<EOT | gpg -a --store | gpg --with-colons pub:1:2:3:$(touch /tmp/bla):5: uid:1:2:3:4:5:6:7:8:Name <Mail> EOT (I haven't checked if this passes all checks, might need changes.) As a side note the code might also not check that the uid is in a line starting with "uid". |
|
|
Was "gpg -a --store" enough for you to be able to reproduce the issue? I haven't tried this one, but the original bug I reported was exploitable in the same way. Ansgar |
|
|
Yep, GPG seems to be one of the uglier parts of the CAcert code, and that is quite an achievement in itself. I have looked at every shell execution in the GPG code now. I hope I didn't miss anything. Please test and review. |
|
|
I create a GPG-key successfully. |
|
|
I tried to add a new gpg-key to an acc. - with non-matching name but matching email - with matching name but non-matching email Both were not accepted. I tried to add a new gpg-key to an acc with matching name and email, but with unchecked CCA. This was rejected with the information to accept CCA. I tried to add a new gpg-key to an acc with matching name and email and accepted CCA. This was accepted. I got a page that showed me the (signed) gpg-key. The key could be found in the list of the gpg-keys of the acc. The key had a new signature. => ok, normal gpg-key-handling works as before. (I did not test any exploit.) |
|
|
The fix has been installed on the production server on October 16, 2013. See also: https://lists.cacert.org/wws/arc/cacert-systemlog/2013-10/msg00005.html |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-08-03 15:39 | ansgar | New Issue | |
| 2013-08-06 21:34 | BenBE | Assigned To | => BenBE |
| 2013-08-06 21:34 | BenBE | Priority | normal => high |
| 2013-08-06 21:34 | BenBE | Severity | major => minor |
| 2013-08-06 21:34 | BenBE | Status | new => confirmed |
| 2013-08-06 21:34 | BenBE | Product Version | => 2013 Q3 |
| 2013-08-06 22:45 | NEOatNHNG | Reviewed by | => NEOatNHNG |
| 2013-08-06 22:45 | NEOatNHNG | Note Added: 0004214 | |
| 2013-08-06 22:45 | NEOatNHNG | Status | confirmed => needs review & testing |
| 2013-08-06 22:50 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 743c8358 |
| 2013-08-06 22:50 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 8fa82f2c |
| 2013-08-07 20:06 | BenBE | Reviewed by | NEOatNHNG => NEOatNHNG, BenBE |
| 2013-08-07 20:06 | BenBE | Note Added: 0004217 | |
| 2013-08-07 20:06 | BenBE | Status | needs review & testing => needs testing |
| 2013-08-25 22:45 | NEOatNHNG | Note Added: 0004252 | |
| 2013-08-25 22:45 | NEOatNHNG | Status | needs testing => ready to deploy |
| 2013-08-29 10:28 | wytze | Note Added: 0004262 | |
| 2013-08-29 10:28 | wytze | Status | ready to deploy => solved? |
| 2013-08-29 10:28 | wytze | Fixed in Version | => 2013 Q3 |
| 2013-08-29 10:28 | wytze | Resolution | open => fixed |
| 2013-09-04 18:33 | ansgar | Note Added: 0004279 | |
| 2013-09-04 18:33 | ansgar | Status | solved? => needs feedback |
| 2013-09-04 18:33 | ansgar | Resolution | fixed => reopened |
| 2013-09-04 18:34 | ansgar | Note View State: 0004279: private | |
| 2013-09-04 18:34 | ansgar | Note View State: 0004279: public | |
| 2013-09-04 21:23 | BenBE | Note Added: 0004286 | |
| 2013-09-04 21:41 | ansgar | Note Added: 0004287 | |
| 2013-09-04 21:41 | ansgar | Status | needs feedback => needs work |
| 2013-09-10 17:48 | Uli60 | Relationship added | related to 0001206 |
| 2013-09-16 19:22 | ansgar | Note Added: 0004332 | |
| 2013-09-17 20:58 | NEOatNHNG | Reviewed by | NEOatNHNG, BenBE => NEOatNHNG |
| 2013-09-17 20:58 | NEOatNHNG | Note Added: 0004337 | |
| 2013-09-17 20:58 | NEOatNHNG | Status | needs work => needs review & testing |
| 2013-09-17 21:00 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 3a0e8ab6 |
| 2013-09-17 21:00 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable dd16a110 |
| 2013-09-17 21:55 | INOPIAE | Note Added: 0004338 | |
| 2013-09-17 22:30 | Eva | Note Added: 0004340 | |
| 2013-09-21 05:21 | BenBE | Reviewed by | NEOatNHNG => NEOatNHNG, BenBE |
| 2013-09-21 05:21 | BenBE | Status | needs review & testing => ready to deploy |
| 2013-10-15 21:20 | BenBE | Source_changeset_attached | => cacert-devel release 02f5da9c |
| 2013-10-16 10:59 | wytze | Note Added: 0004396 | |
| 2013-10-16 10:59 | wytze | Status | ready to deploy => solved? |
| 2013-10-16 10:59 | wytze | Fixed in Version | 2013 Q3 => 2013 Q4 |
| 2013-10-16 10:59 | wytze | Resolution | reopened => fixed |
| 2013-11-20 22:19 | NEOatNHNG | View Status | private => public |
| 2014-02-22 07:20 | INOPIAE | Status | solved? => closed |
