Bug Summary

File:src/usr.sbin/acme-client/keyproc.c
Warning:line 66, column 2
Potential leak of memory pointed to by 'cp'

Annotated Source Code

Press '?' to see keyboard shortcuts

clang -cc1 -cc1 -triple amd64-unknown-openbsd7.0 -analyze -disable-free -disable-llvm-verifier -discard-value-names -main-file-name keyproc.c -analyzer-store=region -analyzer-opt-analyze-nested-blocks -analyzer-checker=core -analyzer-checker=apiModeling -analyzer-checker=unix -analyzer-checker=deadcode -analyzer-checker=security.insecureAPI.UncheckedReturn -analyzer-checker=security.insecureAPI.getpw -analyzer-checker=security.insecureAPI.gets -analyzer-checker=security.insecureAPI.mktemp -analyzer-checker=security.insecureAPI.mkstemp -analyzer-checker=security.insecureAPI.vfork -analyzer-checker=nullability.NullPassedToNonnull -analyzer-checker=nullability.NullReturnedFromNonnull -analyzer-output plist -w -setup-static-analyzer -mrelocation-model pic -pic-level 1 -pic-is-pie -mframe-pointer=all -relaxed-aliasing -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -target-feature +retpoline-indirect-calls -target-feature +retpoline-indirect-branches -tune-cpu generic -debugger-tuning=gdb -fcoverage-compilation-dir=/usr/src/usr.sbin/acme-client/obj -resource-dir /usr/local/lib/clang/13.0.0 -I /usr/src/usr.sbin/acme-client -internal-isystem /usr/local/lib/clang/13.0.0/include -internal-externc-isystem /usr/include -O2 -fdebug-compilation-dir=/usr/src/usr.sbin/acme-client/obj -ferror-limit 19 -fwrapv -D_RET_PROTECTOR -ret-protector -fgnuc-version=4.2.1 -vectorize-loops -vectorize-slp -fno-builtin-malloc -fno-builtin-calloc -fno-builtin-realloc -fno-builtin-valloc -fno-builtin-free -fno-builtin-strdup -fno-builtin-strndup -analyzer-output=html -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /home/ben/Projects/vmm/scan-build/2022-01-12-194120-40624-1 -x c /usr/src/usr.sbin/acme-client/keyproc.c
1/* $Id: keyproc.c,v 1.16 2021/09/17 20:02:24 sthen Exp $ */
2/*
3 * Copyright (c) 2016 Kristaps Dzonsons <kristaps@bsd.lv>
4 *
5 * Permission to use, copy, modify, and distribute this software for any
6 * purpose with or without fee is hereby granted, provided that the above
7 * copyright notice and this permission notice appear in all copies.
8 *
9 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHORS DISCLAIM ALL WARRANTIES
10 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
11 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR
12 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
13 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
14 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
15 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
16 */
17
18#include <sys/stat.h>
19
20#include <err.h>
21#include <stdio.h>
22#include <stdlib.h>
23#include <string.h>
24#include <unistd.h>
25
26#include <openssl/pem.h>
27#include <openssl/err.h>
28#include <openssl/rand.h>
29#include <openssl/x509.h>
30#include <openssl/x509v3.h>
31
32#include "extern.h"
33#include "key.h"
34
35/*
36 * This was lifted more or less directly from demos/x509/mkreq.c of the
37 * OpenSSL source code.
38 */
39static int
40add_ext(STACK_OF(X509_EXTENSION)struct stack_st_X509_EXTENSION *sk, int nid, const char *value)
41{
42 X509_EXTENSION *ex;
43 char *cp;
44
45 /*
46 * XXX: I don't like this at all.
47 * There's no documentation for X509V3_EXT_conf_nid, so I'm not
48 * sure if the "value" parameter is ever written to, touched,
49 * etc.
50 * The 'official' examples suggest not (they use a string
51 * literal as the input), but to be safe, I'm doing an
52 * allocation here and just letting it go.
53 * This leaks memory, but bounded to the number of SANs.
54 */
55
56 if ((cp = strdup(value)) == NULL((void*)0)) {
25
←
Memory is allocated
→
26
←
Assuming the condition is false
→
27
←
Taking false branch
→
57 warn("strdup");
58 return (0);
59 }
60 ex = X509V3_EXT_conf_nid(NULL((void*)0), NULL((void*)0), nid, cp);
61 if (ex == NULL((void*)0)) {
28
←
Assuming 'ex' is not equal to NULL
→
29
←
Taking false branch
→
62 warnx("X509V3_EXT_conf_nid");
63 free(cp);
64 return (0);
65 }
66 sk_X509_EXTENSION_push(sk, ex)sk_push(((_STACK*) (1 ? (sk) : (struct stack_st_X509_EXTENSION
*)0)), ((void*) (1 ? (ex) : (X509_EXTENSION*)0)));
30
←
Potential leak of memory pointed to by 'cp'
67 return (1);
68}
69
70/*
71 * Create an X509 certificate from the private key we have on file.
72 * To do this, we first open the key file, then jail ourselves.
73 * We then use the crypto library to create the certificate within the
74 * jail and, on success, ship it to "netsock" as an X509 request.
75 */
76int
77keyproc(int netsock, const char *keyfile, const char **alts, size_t altsz,
78 enum keytype keytype)
79{
80 char *der64 = NULL((void*)0), *der = NULL((void*)0), *dercp;
81 char *sans = NULL((void*)0), *san = NULL((void*)0);
82 FILE *f;
83 size_t i, sansz;
84 void *pp;
85 EVP_PKEY *pkey = NULL((void*)0);
86 X509_REQ *x = NULL((void*)0);
87 X509_NAME *name = NULL((void*)0);
88 int len, rc = 0, cc, nid, newkey = 0;
89 mode_t prev;
90 STACK_OF(X509_EXTENSION)struct stack_st_X509_EXTENSION *exts = NULL((void*)0);
91
92 /*
93 * First, open our private key file read-only or write-only if
94 * we're creating from scratch.
95 * Set our umask to be maximally restrictive.
96 */
97
98 prev = umask((S_IWUSR0000200 | S_IXUSR0000100) | S_IRWXG0000070 | S_IRWXO0000007);
99 if ((f = fopen(keyfile, "r")) == NULL((void*)0) && errno(*__errno()) == ENOENT2) {
1
Assuming the condition is false
→
100 f = fopen(keyfile, "wx");
101 newkey = 1;
102 }
103 umask(prev);
104
105 if (f
1.1
'f' is not equal to NULL
 == NULL((void*)0)) {
2
←
Taking false branch
→
106 warn("%s", keyfile);
107 goto out;
108 }
109
110 /* File-system, user, and sandbox jail. */
111
112 ERR_load_crypto_strings();
113
114 if (pledge("stdio", NULL((void*)0)) == -1) {
3
←
Assuming the condition is false
→
4
←
Taking false branch
→
115 warn("pledge");
116 goto out;
117 }
118
119 if (newkey
4.1
'newkey' is 0
) {
5
←
Taking false branch
→
120 switch (keytype) {
121 case KT_ECDSA:
122 if ((pkey = ec_key_create(f, keyfile)) == NULL((void*)0))
123 goto out;
124 dodbg("%s: generated ECDSA domain key", keyfile);
125 break;
126 case KT_RSA:
127 if ((pkey = rsa_key_create(f, keyfile)) == NULL((void*)0))
128 goto out;
129 dodbg("%s: generated RSA domain key", keyfile);
130 break;
131 }
132 } else {
133 if ((pkey = key_load(f, keyfile)) == NULL((void*)0))
6
←
Assuming the condition is false
→
7
←
Taking false branch
→
134 goto out;
135 /* XXX check if domain key type equals configured key type */
136 doddbg("%s: loaded domain key", keyfile);
137 }
138
139 fclose(f);
140 f = NULL((void*)0);
141
142 /*
143 * Generate our certificate from the EVP public key.
144 * Then set it as the X509 requester's key.
145 */
146
147 if ((x = X509_REQ_new()) == NULL((void*)0)) {
8
←
Assuming the condition is false
→
9
←
Taking false branch
→
148 warnx("X509_new");
149 goto out;
150 } else if (!X509_REQ_set_pubkey(x, pkey)) {
10
←
Assuming the condition is false
→
11
←
Taking false branch
→
151 warnx("X509_set_pubkey");
152 goto out;
153 }
154
155 /* Now specify the common name that we'll request. */
156
157 if ((name = X509_NAME_new()) == NULL((void*)0)) {
12
←
Assuming the condition is false
→
13
←
Taking false branch
→
158 warnx("X509_NAME_new");
159 goto out;
160 } else if (!X509_NAME_add_entry_by_txt(name, "CN",
14
←
Assuming the condition is false
→
15
←
Taking false branch
→
161 MBSTRING_ASC(0x1000|1), (u_char *)alts[0], -1, -1, 0)) {
162 warnx("X509_NAME_add_entry_by_txt: CN=%s", alts[0]);
163 goto out;
164 } else if (!X509_REQ_set_subject_name(x, name)) {
16
←
Assuming the condition is false
→
17
←
Taking false branch
→
165 warnx("X509_req_set_issuer_name");
166 goto out;
167 }
168
169 /*
170 * Now add the SAN extensions.
171 * This was lifted more or less directly from demos/x509/mkreq.c
172 * of the OpenSSL source code.
173 * (The zeroth altname is the domain name.)
174 * TODO: is this the best way of doing this?
175 */
176
177 nid = NID_subject_alt_name85;
178 if ((exts = sk_X509_EXTENSION_new_null()((struct stack_st_X509_EXTENSION *)sk_new_null())) == NULL((void*)0)) {
18
←
Assuming the condition is false
→
19
←
Taking false branch
→
179 warnx("sk_X509_EXTENSION_new_null");
180 goto out;
181 }
182 /* Initialise to empty string. */
183 if ((sans = strdup("")) == NULL((void*)0)) {
20
←
Assuming the condition is false
→
21
←
Taking false branch
→
184 warn("strdup");
185 goto out;
186 }
187 sansz = strlen(sans) + 1;
188
189 /*
190 * For each SAN entry, append it to the string.
191 * We need a single SAN entry for all of the SAN
192 * domains: NOT an entry per domain!
193 */
194
195 for (i = 0; i < altsz; i++) {
22
←
Assuming 'i' is >= 'altsz'
→
23
←
Loop condition is false. Execution continues on line 214
→
196 cc = asprintf(&san, "%sDNS:%s",
197 i ? "," : "", alts[i]);
198 if (cc == -1) {
199 warn("asprintf");
200 goto out;
201 }
202 pp = recallocarray(sans, sansz, sansz + strlen(san), 1);
203 if (pp == NULL((void*)0)) {
204 warn("recallocarray");
205 goto out;
206 }
207 sans = pp;
208 sansz += strlen(san);
209 strlcat(sans, san, sansz);
210 free(san);
211 san = NULL((void*)0);
212 }
213
214 if (!add_ext(exts, nid, sans)) {
24
←
Calling 'add_ext'
→
215 warnx("add_ext");
216 goto out;
217 } else if (!X509_REQ_add_extensions(x, exts)) {
218 warnx("X509_REQ_add_extensions");
219 goto out;
220 }
221 sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free)sk_pop_free(((_STACK*) (1 ? (exts) : (struct stack_st_X509_EXTENSION
*)0)), ((void (*)(void *)) ((1 ? (X509_EXTENSION_free) : (void
(*)(X509_EXTENSION *))0))));
222
223 /* Sign the X509 request using SHA256. */
224
225 if (!X509_REQ_sign(x, pkey, EVP_sha256())) {
226 warnx("X509_sign");
227 goto out;
228 }
229
230 /* Now, serialise to DER, then base64. */
231
232 if ((len = i2d_X509_REQ(x, NULL((void*)0))) < 0) {
233 warnx("i2d_X509");
234 goto out;
235 } else if ((der = dercp = malloc(len)) == NULL((void*)0)) {
236 warn("malloc");
237 goto out;
238 } else if (len != i2d_X509_REQ(x, (u_char **)&dercp)) {
239 warnx("i2d_X509");
240 goto out;
241 } else if ((der64 = base64buf_url(der, len)) == NULL((void*)0)) {
242 warnx("base64buf_url");
243 goto out;
244 }
245
246 /*
247 * Write that we're ready, then write.
248 * We ignore reader-closed failure, as we're just going to roll
249 * into the exit case anyway.
250 */
251
252 if (writeop(netsock, COMM_KEY_STAT, KEY_READY) < 0)
253 goto out;
254 if (writestr(netsock, COMM_CERT, der64) < 0)
255 goto out;
256
257 rc = 1;
258out:
259 close(netsock);
260 if (f != NULL((void*)0))
261 fclose(f);
262 free(der);
263 free(der64);
264 free(sans);
265 free(san);
266 X509_REQ_free(x);
267 X509_NAME_free(name);
268 EVP_PKEY_free(pkey);
269 ERR_print_errors_fp(stderr(&__sF[2]));
270 ERR_free_strings();
271 return rc;
272}