mirror of
git://git.gnupg.org/gnupg.git
synced 2024-12-22 10:19:57 +01:00
fdd1567743
* g10/call-agent.c (agent_pkdecrypt): accept but do not require NUL-terminated data from the agent. * sm/call-agent.c (gpgsm_agent_pkdecrypt): accept but do not require NUL-terminated data from the agent. -- The current code for both gpg and gpgsm assumes that gpg-agent will return string terminated with a single NUL, even though the string that it receives is also already length-delimited. Since these tools might be talking to an older version of gpg-agent, we want to continue to make sense of such a response, but we really shouldn't depend on it. Rather, we can just strip off all trailing NULs and then treat the remaining string as a proper S-expression. We can't assume tha the S-expression itself is a NUL-terminated string, because any of the canonically-represented objects could contain a NUL byte internally. But if it's a proper S-expression, then it must actually terminate in a non-NUL ')' octet. I note that gpgsm_agent_pkdecrypt() appears to try to work with older versions of gpg-agent which might not return a full S-expression. This makes it harder to reason about, since a maliciously-formed return value could contain a string that could cause invalid memory access when invoking strtoul (e.g. all numbers up to the end of the buffer). So we still have to manually NUL-terminate it before continuing in that codepath. This cleanup would be easier if we could just assume that the agent will always return an S-expression. Perhaps that could be a subsequent cleanup for gpgsm? Do we expect all versions of gpgsm to interoperate with all past versions of gpg-agent? gpg's agent_pkdecrypt() has no such qualms -- if the returned object is not a full S-expression, then it rejects the response. This makes it much easier to reason about the pkdecrypt response without modification, and allows us to strip any trailing NUL bytes knowing that the response string will be properly terminated with a close parenthesis. GnuPG-bug-id: 4652 Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>