* agent/command-ssh.c (ssh_search_control_file): Check S before
upcasing it.
--
In contradiction to the comment we did not check the length of HEXGRIP
and thus the GPG_ERR_INV_LENGTH was never triggered.
Detected by Stack 0.3:
bug: anti-simplify
model: |
%cmp8 = icmp ne i32 %i.0, 40, !dbg !986
--> false
stack:
- /home/wk/s/gnupg/agent/command-ssh.c:1226:0
ncore: 2
core:
- /home/wk/s/gnupg/agent/command-ssh.c:1225:0
- buffer overflow
- /home/wk/s/gnupg/agent/command-ssh.c:1225:0
- buffer overflow
(backported from 2.1 commit 3529dd8bb5bafc4e02915648d5f409bd27a9cc37)
* g10/parse-packet.c (parse_key): Check PKTLEN before calling mpi_read
et al.
--
Due to the missing length checks PKTLEN may turn negative. Because
PKTLEN is an unsigned int the malloc in read_rest would try to malloc
a too large number and terminate the process with "error reading rest
of packet: Cannot allocate memory".
Reported-by: Hanno Böck.
Signed-off-by: Werner Koch <wk@gnupg.org>
(backported from 2.1 commit d901efcebaefaf6eae4a9b9aa8f0c2c055d3518a)
* tools/gpgtar-extract.c (extract_regular): Handle size multiples
of RECORDSIZE.
--
If a hdr->size was a multiple of 512 the last record would
not have been written and the files corrupted accordingly.
GnuPG-bug-id: 1926
Signed-off-by: Andre Heinecke <aheinecke@intevation.de>
Changed to use only if-else.
Signed-off-by: Werner Koch <wk@gnupg.org>
(cherry picked from commit 6cbbb0bec98e1acefc4c7163cc41a507469db920)
* common/argparse.h (ARGPARSE_INVALID_ARG): New.
* common/argparse.c: Include limits h and errno.h.
(initialize): Add error strings for new error constant.
(set_opt_arg): Add range checking.
--
Signed-off-by: Werner Koch <wk@gnupg.org>
[ This is a backport of 0d73a242cb53522669cf712b5ece7d1ed05d003a from
master to STABLE-BRANCH-2-0 ]
Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
* keyserver/curl-shim.c (curl_easy_setopt) : ensure that va_end is
called.
--
stdarg(3) says:
Each invocation of va_start() must be matched by a
corresponding invocation of va_end() in the same function.
Observed by Joshua Rogers <honey@internot.info>
Debian-Bug-Id: #773475
* g10/armor.c (parse_key_failed_line): New.
(check_input): Watch out for gpgkeys_ error lines.
* g10/filter.h (armor_filter_context_t): Add field key_failed_code.
* g10/import.c (import): Add arg r_gpgkeys_err.
(import_keys_internal): Ditto.
(import_keys_stream): Ditto.
* g10/keyserver.c (keyserver_errstr): New.
(keyserver_spawn): Detect "KEY " lines while sending. Get gpgkeys_err
while receiving keys.
(keyserver_work): Add kludge for better error messages.
--
GnuPG-bug-id: 1832
Note that these changes can be backported to 1.4 but they don't make
sense for 2.1 due to the removal of the keyserver helpers. The error
reporting could be improved even more but given that this is an old
GnuPG branch it is not justified to put too much effort into it.
Signed-off-by: Werner Koch <wk@gnupg.org>
* include/host2net.h (buf16_to_ulong, buf16_to_uint): New.
(buf16_to_ushort, buf16_to_u16): New.
(buf32_to_size_t, buf32_to_ulong, buf32_to_uint, buf32_to_u32): New.
--
This fixes sign extension on shift problems. Hanno Böck found a case
with an invalid read due to this problem. To fix that almost all uses
of "<< 24" and "<< 8" are changed by this patch to use an inline
function from host2net.h.
(back ported from commit 2183683bd633818dd031b090b5530951de76f392)
Signed-off-by: Werner Koch <wk@gnupg.org>
* g10/keyring.c (keyring_get_keyblock): Whitelist allowed packet
types.
--
The keyring DB code did not reject packets which don't belong into a
keyring. If for example the keyblock contains a literal data packet
it is expected that the processing code stops at the data packet and
reads from the input stream which is referenced from the data packets.
Obviously the keyring processing code does not and cannot do that.
However, when exporting this messes up the IOBUF and leads to an
invalid read of sizeof (int).
We now skip all packets which are not allowed in a keyring.
Reported-by: Hanno Böck <hanno@hboeck.de>
(back ported from commit f0f71a721ccd7ab9e40b8b6b028b59632c0cc648)
* g10/build-packet.c (write_fake_data): Take care of a NULL stored as
opaque MPI.
--
Reported-by: Hanno Böck <hanno@hboeck.de>
(back ported from commit 0835d2f44ef62eab51fce6a927908f544e01cf8f)
* kbx/keybox-update.c (blob_filecopy): Fix resource leak. On error
return, 'fp' and 'newfp' was never closed.
--
Signed-off-by: Joshua Rogers <git@internot.info>
[Log entry reformatted, and added more fixes - gniibe]
(cherry picked from commit 7db6c82cec49b7c56c403a8ea98364086baf75f3)
* g10/parse-packet.c (MAX_KEY_PACKET_LENGTH): New.
(MAX_UID_PACKET_LENGTH): New.
(MAX_COMMENT_PACKET_LENGTH): New.
(MAX_ATTR_PACKET_LENGTH): New.
(parse_key): Limit the size of a key packet to 256k.
(parse_user_id): Use macro for the packet size limit.
(parse_attribute): Ditto.
(parse_comment): Ditto.
--
Without that it is possible to force gpg to allocate large amounts of
memory by using a bad encoded MPI. This would be an too easy DoS.
Another way to mitigate would be to change the MPI read function to
allocate memory dynamically while reading the MPI. However, that
complicates and possibly slows down the code. A too large key packet
is in any case a sign for broken data and thus gpg should not use it.
Reported-by: Hanno Böck
GnuPG-bug-id: 1823
Signed-off-by: Werner Koch <wk@gnupg.org>
(back ported from commit 382ba4b137b42d5f25a7e256bb7c053ee5ac7b64)
* jnlib/dotlock.c (create_dotlock): Avoid double close due to EINTR.
--
close(2) says:
close() should not be retried after an EINTR since this may cause a
reused descriptor from another thread to be closed.
(backported from commit 628b111fa679612e23c0d46505b1ecbbf091897d)
Debian-Bug-Id: 773423
Signed-off-by: Werner Koch <wk@gnupg.org>
* g10/keygen.c (ask_algo): Add list of strings.
--
Signed-off-by: Werner Koch <wk@gnupg.org>
(backported from commit b1d5ed6ac842469afcb84868d0f6641dc286a6c7)
* g10/keyedit.c (subkey_expire_warning): New.
keyedit_menu): Call it when needed.
--
GnuPG-bug-id: 1715
The heuristic to detect a problem is not very advanced but it should
catch the most common cases.
(backported from commit ae3d1bbb65b65cf3c57bb14886be120f5e31635d)
* scd/ccid-driver.c (send_escape_cmd): Fix setting of 'rc'.
--
Variable 'rc' in send_escape_cmd was overwritten before it was
returned, leading to incorrect computation.
Signed-off-by: Joshua Rogers <git@internot.info>
[Log entry reformatted - wk]
* tools/gpgconf-comp.c (option_check_validity): Enable check for
UINT32.
--
Reported-by: Günther Noack <gnoack@google.com>
This is actually a bug which inhibited the checking of values of type
UINT32.
Signed-off-by: Werner Koch <wk@gnupg.org>
(cherry picked from commit 3f6abb57a7b5e54b593c5775c8f7a07d61119705)
* sm/minip12.c: (p12_build) if jnlib_iconv_open fails, avoid
double-free of pwbuf.
--
Observed by Joshua Rogers <honey@internot.info>, who proposed a
slightly different fix.
Debian-Bug-Id: 773472
Added fix at a second place - wk.
* scd/command.c (cmd_readkey): avoid double-free of cert
--
When ksba_cert_new() fails, cert will be double-freed.
Debian-Bug-Id: 773471
Original patch changed by wk to do the free only at leave.
* common/iobuf.c: (iobuf_open): initialize len
--
In iobuf_open, IOBUFCTRL_DESC and IOBUFCTRL_INIT commands are invoked
(via file_filter()) on fcx, passing in a pointer to an uninitialized
len.
With these two commands, file_filter doesn't actually do anything with
the value of len, so there's no actual risk of use of uninitialized
memory in the code as it stands.
However, some static analysis tools might flag this situation with a
warning, and initializing the value doesn't hurt anything, so i think
this trivial cleanup is warranted.
Debian-Bug-Id: 773469
* tools/gpgkey2ssh.c (key_to_blob) : ensure that va_end is called.
--
stdarg(3) says:
Each invocation of va_start() must be matched by a
corresponding invocation of va_end() in the same function.
Observed by Joshua Rogers <honey@internot.info>
Debian-Bug-Id: 773415
* sm/gpgsm.c (parse_keyserver_line): Set SERVER to NULL.
--
Cherry-pick of abd5f6752d693b7f313c19604f0723ecec4d39a6.
Reported-by: Joshua Rogers <git@internot.info>
"If something inside the ldapserver_parse_one function failed,
'server' would be freed, then returned, leading to a
use-after-free. This code is likely copied from sm/gpgsm.c, which
was also susceptible to this bug."
Signed-off-by: Werner Koch <wk@gnupg.org>
* g10/parse-packet.c (can_handle_critical): Check content length
before calling can_handle_critical_notation.
--
The problem was found by Jan Bee and gniibe proposed the used fix.
Thanks.
This bug can't be exploited: Only if the announced length of the
notation is 21 or 32 a memcmp against fixed strings using that length
would be done. The compared data is followed by the actual signature
and thus it is highly likely that not even read of unallocated memory
will happen. Nevertheless such a bug needs to be fixed.
Signed-off-by: Werner Koch <wk@gnupg.org>
* scd/app-openpgp.c (get_public_key): correctly close 'fp' upon use.
--
Inside the get_public_key function, 'fp' was opened using popen, but
incorrectly closed using fclose.
Debian-Bug-Id: 773474
* g10/keygen.c (generate_subkeypair): Release DEK soon.
--
This fixes the out_of_core error in the test case of adding
RSA-4096 subkey to RSA-4096 primary key with configuration:
s2k-cipher-algo S10
Debian-bug-id: 772780
Cherry-picked da66ad5bba4215b9ddd0cb927a89aa75355632aa from
STABLE-BRANCH-1-4 branch.
