New Defects reported by Coverity Scan for sbabic/swupdate

148 views
Skip to first unread message

scan-...@coverity.com

unread,
Oct 11, 2021, 10:51:41 AM10/11/21
to swup...@googlegroups.com
Hi,

Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.

2 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 2 of 2 defect(s)


** CID 339139: Error handling issues (CHECKED_RETURN)
/core/cpio_utils.c: 643 in __swupdate_copy()


________________________________________________________________________________________________________
*** CID 339139: Error handling issues (CHECKED_RETURN)
/core/cpio_utils.c: 643 in __swupdate_copy()
637 ret = -EFAULT;
638 goto copyfile_exit;
639 }
640 }
641
642 if (!inbuf)
>>> CID 339139: Error handling issues (CHECKED_RETURN)
>>> Calling "fill_buffer" without checking return value (as is done elsewhere 4 out of 5 times).
643 fill_buffer(fdin, buffer, NPAD_BYTES(*offs), offs, checksum, NULL);
644
645 if (checksum != NULL) {
646 *checksum = input_state.checksum;
647 }
648

** CID 339138: Null pointer dereferences (NULL_RETURNS)
/corelib/multipart_parser.c: 87 in multipart_parser_init()


________________________________________________________________________________________________________
*** CID 339138: Null pointer dereferences (NULL_RETURNS)
/corelib/multipart_parser.c: 87 in multipart_parser_init()
81 (const char *boundary, const multipart_parser_settings* settings) {
82
83 multipart_parser* p = malloc(sizeof(multipart_parser) +
84 strlen(boundary) +
85 strlen(boundary) + 9);
86
>>> CID 339138: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "p->multipart_boundary" when calling "strcpy".
87 strcpy(p->multipart_boundary, boundary);
88 p->boundary_length = strlen(boundary);
89
90 p->lookbehind = (p->multipart_boundary + p->boundary_length + 1);
91
92 p->index = 0;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrffGusdB2gY411RFCZULj23zVH-2FYjULe-2FZVatHaTNOtXGWK7d76ThnungrCH2R2Ak-3DwEug_U-2BcIo0IeFG9EIIQe7AiEg4oWkKw9GoOEX2k5mfx97s-2BSlMv7Q3QmUSjaRzvgESzfiAIcGIX63Yq38b86GdbpunJ4csgEjUKXRUFB7aT-2Bk3Jwx5S8-2Fp4RvdPuoUnfWpsbF3SifOtts6KhoEHIt6DCsViytZ0w-2Ftg4Y2Q9mkBVgqe21MKPSZp689e0XldNQt-2F2lMNldZaKELHzH8OytwW4aT1bIlAqLkAUZ-2B5KcXIJ-2BZw-3D

scan-...@coverity.com

unread,
Oct 18, 2021, 9:06:57 AM10/18/21
to swup...@googlegroups.com
Hi,

Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.

1 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 340471: Resource leaks (RESOURCE_LEAK)
/core/artifacts_versions.c: 175 in version_to_number()


________________________________________________________________________________________________________
*** CID 340471: Resource leaks (RESOURCE_LEAK)
/core/artifacts_versions.c: 175 in version_to_number()
169 if (count < 4) {
170 unsigned long int fld = strtoul(*ver, NULL, 10);
171 /* check for return of strtoul, mandatory */
172 if (fld > 0xffff) {
173 TRACE("Version %s had an element > 65535, falling back to semver",
174 version_string);
>>> CID 340471: Resource leaks (RESOURCE_LEAK)
>>> Variable "versions" going out of scope leaks the storage it points to.
175 return false;
176 }
177 version = (version << 16) | fld;
178 }
179 free(*ver);
180 }


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrffGusdB2gY411RFCZULj23zVH-2FYjULe-2FZVatHaTNOtXGWK7d76ThnungrCH2R2Ak-3DN0FU_U-2BcIo0IeFG9EIIQe7AiEg4oWkKw9GoOEX2k5mfx97s897w42X7szsrSCRHFyR4aGeMOVcj-2FKeaeSeu-2BYKQ3UTH3u6Bz85XSOdjN9gehBSYsPPeKH9tvnrdolzzrv0DgvL-2B58brYFrhu-2FPvjx7Qy-2BSUNr-2BF3EGB91FWGd0p91C7NqXptYCkqI-2BMZr6DIDTb7HDPTqSfK02wSvfPJwTiofgqrVssRy2r2m1kAeFORdw9g-3D

Stefano Babic

unread,
Oct 18, 2021, 9:39:03 AM10/18/21
to swup...@googlegroups.com, Dominique MARTINET
Hi Dominique,

coverity lokks to be right:

On 18.10.21 15:06, scan-admin via swupdate wrote:
> Hi,
>
> Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.
>
> 1 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 1 of 1 defect(s)
>
>
> ** CID 340471: Resource leaks (RESOURCE_LEAK)
> /core/artifacts_versions.c: 175 in version_to_number()
>
>
> ________________________________________________________________________________________________________
> *** CID 340471: Resource leaks (RESOURCE_LEAK)
> /core/artifacts_versions.c: 175 in version_to_number()
> 169 if (count < 4) {
> 170 unsigned long int fld = strtoul(*ver, NULL, 10);
> 171 /* check for return of strtoul, mandatory */
> 172 if (fld > 0xffff) {
> 173 TRACE("Version %s had an element > 65535, falling back to semver",
> 174 version_string);
>>>> CID 340471: Resource leaks (RESOURCE_LEAK)
>>>> Variable "versions" going out of scope leaks the storage it points to.

Yes, versions is not freed, this is a memory leak.

> 175 return false;
> 176 }
> 177 version = (version << 16) | fld;
> 178 }
> 179 free(*ver);

Anyway, I am also looking at this one and to the later free(versions),
and it looks I have originally weird implemented. Even if I do not
recognize a memory leak, I have already added an accessor to free all
memory. That is, instead of calling free(*ver) in loop, I should better
call free_string_array(versions) before returning. What do you think ? 4
eyes are often better as two...


Regards,
Stefano



--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

Dominique MARTINET

unread,
Oct 18, 2021, 7:31:14 PM10/18/21
to Stefano Babic, swup...@googlegroups.com
ah, yes, sorry I had noticed the weird pattern in the first version of
my patch but did not think of early return here.
I'm not writing enough C nowadays..


Let's use free_string_array(versions), it's much simpler, and can
simplify the if (count < 4) sub-expression further.
I'm sending a v2 in reply to this message, here's the diff between v1
and v2 if you prefer reading that:
------------
diff --git a/core/artifacts_versions.c b/core/artifacts_versions.c
index e8dac70c45e0..5b747dfd73f7 100644
--- a/core/artifacts_versions.c
+++ b/core/artifacts_versions.c
@@ -162,21 +162,19 @@ static bool version_to_number(const char *version_string, __u64 *version_number)
char **versions = NULL;
char **ver;
unsigned int count = 0;
+ bool valid = false;
__u64 version = 0;

versions = string_split(version_string, '.');
- for (ver = versions; *ver != NULL; ver++, count++) {
- if (count < 4) {
- unsigned long int fld = strtoul(*ver, NULL, 10);
- /* check for return of strtoul, mandatory */
- if (fld > 0xffff) {
- TRACE("Version %s had an element > 65535, falling back to semver",
- version_string);
- return false;
- }
- version = (version << 16) | fld;
+ for (ver = versions; *ver != NULL && count < 4; ver++, count++) {
+ unsigned long int fld = strtoul(*ver, NULL, 10);
+ /* check for return of strtoul, mandatory */
+ if (fld > 0xffff) {
+ TRACE("Version %s had an element > 65535, falling back to semver",
+ version_string);
+ goto out;
}
- free(*ver);
+ version = (version << 16) | fld;
}
if (count >= 4) {
TRACE("Version %s had more than 4 numbers, trailing numbers will be ignored",
@@ -184,10 +182,13 @@ static bool version_to_number(const char *version_string, __u64 *version_number)
} else if (count > 0) {
version <<= 16 * (4 - count);
}
- free(versions);
-
*version_number = version;
- return true;
+ valid = true;
+
+out:
+ free_string_array(versions);
+
+ return valid;
}

static const char ACCEPTED_CHARS[] = "0123456789.";
------------
--
Dominique

Dominique Martinet

unread,
Oct 18, 2021, 7:31:52 PM10/18/21
to swup...@googlegroups.com, Stefano Babic, Dominique Martinet
is_oldstyle_version would accept any string comprised of digits and dots.
Parse the actual version in is_oldstyle_version to detect large numbers and
fall back to semver if a large number was given, as semver can handle bigger
values.
Note that if this happens on the 4th level, this result in that number
actually being ignored as semver only handles up to major.minor.patch
levels.

Also print trace messages in case of parse failure (too many digits or number
too big) to facilitate debugging.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---
v1->v2:
- fix memory leak on early return
- use free_string_array()

core/artifacts_versions.c | 74 +++++++++++++++++++++++----------------
1 file changed, 43 insertions(+), 31 deletions(-)

diff --git a/core/artifacts_versions.c b/core/artifacts_versions.c
index 433125862014..e93248d8b955 100644
--- a/core/artifacts_versions.c
+++ b/core/artifacts_versions.c
@@ -147,19 +147,6 @@ void get_sw_versions(swupdate_cfg_handle __attribute__ ((__unused__))*handle,
}
#endif

-static const char ACCEPTED_CHARS[] = "0123456789.";
-
-static bool is_oldstyle_version(const char* version_string)
-{
- while (*version_string)
- {
- if (strchr(ACCEPTED_CHARS, *version_string) == NULL)
- return false;
- ++version_string;
- }
- return true;
-}
-
/*
* convert a version string into a number
* version string is in the format:
@@ -170,30 +157,52 @@ static bool is_oldstyle_version(const char* version_string)
* Also major.minor or major.minor.revision are allowed
* The conversion generates a 64 bit value that can be compared
*/
-static __u64 version_to_number(const char *version_string)
+static bool version_to_number(const char *version_string, __u64 *version_number)
{
char **versions = NULL;
char **ver;
unsigned int count = 0;
+ bool valid = false;
__u64 version = 0;

versions = string_split(version_string, '.');
- for (ver = versions; *ver != NULL; ver++, count ++) {
- if (count < 4) {
- unsigned long int fld = strtoul(*ver, NULL, 10);
- /* check for return of strtoul, mandatory */
- if (fld != ULONG_MAX) {
- fld &= 0xffff;
- version = (version << 16) | fld;
- }
+ for (ver = versions; *ver != NULL && count < 4; ver++, count++) {
+ unsigned long int fld = strtoul(*ver, NULL, 10);
+ /* check for return of strtoul, mandatory */
+ if (fld > 0xffff) {
+ TRACE("Version %s had an element > 65535, falling back to semver",
+ version_string);
+ goto out;
}
- free(*ver);
+ version = (version << 16) | fld;
}
- if ((count < 4) && (count > 0))
+ if (count >= 4) {
+ TRACE("Version %s had more than 4 numbers, trailing numbers will be ignored",
+ version_string);
+ } else if (count > 0) {
version <<= 16 * (4 - count);
- free(versions);
+ }
+ *version_number = version;
+ valid = true;

- return version;
+out:
+ free_string_array(versions);
+
+ return valid;
+}
+
+static const char ACCEPTED_CHARS[] = "0123456789.";
+
+static bool is_oldstyle_version(const char *version_string, __u64 *version_number)
+{
+ const char *ver = version_string;
+ while (*ver)
+ {
+ if (strchr(ACCEPTED_CHARS, *ver) == NULL)
+ return false;
+ ++ver;
+ }
+ return version_to_number(version_string, version_number);;
}

/*
@@ -209,12 +218,15 @@ static __u64 version_to_number(const char *version_string)
*/
int compare_versions(const char* left_version, const char* right_version)
{
- if (is_oldstyle_version(left_version) && is_oldstyle_version(right_version))
- {
- __u64 left_u64 = version_to_number(left_version);
- __u64 right_u64 = version_to_number(right_version);

- DEBUG("Comparing old-style versions '%s' <-> '%s'", left_version, right_version);
+ __u64 left_u64;
+ __u64 right_u64;
+
+ if (is_oldstyle_version(left_version, &left_u64)
+ && is_oldstyle_version(right_version, &right_u64))
+ {
+ DEBUG("Comparing old-style versions '%s' <-> '%s'",
+ left_version, right_version);
TRACE("Parsed: '%llu' <-> '%llu'", left_u64, right_u64);

if (left_u64 < right_u64)
--
2.30.2

Dominique Martinet

unread,
Oct 18, 2021, 7:42:25 PM10/18/21
to swup...@googlegroups.com, Stefano Babic
Dominique Martinet wrote on Tue, Oct 19, 2021 at 08:31:42AM +0900:
> + if (fld > 0xffff) {
> + TRACE("Version %s had an element > 65535, falling back to semver",
> + version_string);

ah, sorry I forgot to make these DEBUG as you had said you did.

I do not see the patch in the coverity_scan branch (or otherwise) so
leaving that part up to you.


BTW I've also looked at your documentation update already in master, it
looks good to me -- thanks!

--
Dominique

Stefano Babic

unread,
Oct 19, 2021, 6:33:51 AM10/19/21
to Dominique Martinet, swup...@googlegroups.com, Stefano Babic
Hi Dominique,

On 19.10.21 01:42, Dominique Martinet wrote:
> Dominique Martinet wrote on Tue, Oct 19, 2021 at 08:31:42AM +0900:
>> + if (fld > 0xffff) {
>> + TRACE("Version %s had an element > 65535, falling back to semver",
>> + version_string);
>
> ah, sorry I forgot to make these DEBUG as you had said you did.

No problem, I fixed it myself.

>
> I do not see the patch in the coverity_scan branch (or otherwise) so
> leaving that part up to you.

I use a gitlab server for these tests, it is also public and I set up a
gitlab runner. Access is public for both sources (but well, it is just a
mirror..) and pipelines:

https://source.denx.de/swupdate

I push coverity only on this server after travis changed conditions and
it is less suitable for FOSS projects. Travis is not used anymore for CI.

Your patch is tested here:

https://source.denx.de/swupdate/swupdate/-/jobs/337560

Pipelines here:

https://source.denx.de/swupdate/swupdate/-/pipelines

The coverity_branch is just a "test" branch to run then coverity tool.
It is not guaranteed that this branch is not rebased (it is a lot). If
everything is fine, I merge then coverity into master and I publish it.

Status of coverity is here:

https://scan.coverity.com/projects/sbabic-swupdate?tab=overview

I tried to make everything open, so that everybody can access.

>
>
> BTW I've also looked at your documentation update already in master, it
> looks good to me -- thanks!

;-)

Best regards,

scan-...@coverity.com

unread,
Oct 25, 2021, 10:24:56 AM10/25/21
to swup...@googlegroups.com
Hi,

Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.

17 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.
21 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 17 of 17 defect(s)


** CID 340737: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 4192 in mg_file_write()


________________________________________________________________________________________________________
*** CID 340737: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 4192 in mg_file_write()
4186 fp = fopen(tmp, "wb");
4187 if (fp != NULL) {
4188 result = fwrite(buf, 1, len, fp) == len;
4189 fclose(fp);
4190 if (result) {
4191 remove(path);
>>> CID 340737: Error handling issues (CHECKED_RETURN)
>>> Calling "rename(tmp, path)" without checking return value. This library function may fail and return an error code.
4192 rename(tmp, path);
4193 } else {
4194 remove(tmp);
4195 }
4196 }
4197 return result;

** CID 340736: (CHECKED_RETURN)
/mongoose/mongoose.c: 3162 in setsockopts()
/mongoose/mongoose.c: 3154 in setsockopts()
/mongoose/mongoose.c: 3152 in setsockopts()
/mongoose/mongoose.c: 3150 in setsockopts()
/mongoose/mongoose.c: 3157 in setsockopts()
/mongoose/mongoose.c: 3163 in setsockopts()


________________________________________________________________________________________________________
*** CID 340736: (CHECKED_RETURN)
/mongoose/mongoose.c: 3162 in setsockopts()
3156 int idle = 60;
3157 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle));
3158 #endif
3159 #if !defined(_WIN32) && !defined(__QNX__)
3160 {
3161 int cnt = 3, intvl = 20;
>>> CID 340736: (CHECKED_RETURN)
>>> Calling "setsockopt((SOCKET)(size_t)c->fd, IPPROTO_TCP, 6, &cnt, 4U)" without checking return value. This library function may fail and return an error code.
3162 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPCNT, &cnt, sizeof(cnt));
3163 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPINTVL, &intvl, sizeof(intvl));
3164 }
3165 #endif
3166 #endif
3167 }
/mongoose/mongoose.c: 3154 in setsockopts()
3148 #define SOL_TCP IPPROTO_TCP
3149 #endif
3150 setsockopt(FD(c), SOL_TCP, TCP_NODELAY, (char *) &on, sizeof(on));
3151 #if defined(TCP_QUICKACK)
3152 setsockopt(FD(c), SOL_TCP, TCP_QUICKACK, (char *) &on, sizeof(on));
3153 #endif
>>> CID 340736: (CHECKED_RETURN)
>>> Calling "setsockopt((SOCKET)(size_t)c->fd, 1, 9, (char *)&on, 4U)" without checking return value. This library function may fail and return an error code.
3154 setsockopt(FD(c), SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on));
3155 #if ESP32 || ESP8266 || defined(__linux__)
3156 int idle = 60;
3157 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle));
3158 #endif
3159 #if !defined(_WIN32) && !defined(__QNX__)
/mongoose/mongoose.c: 3152 in setsockopts()
3146 int on = 1;
3147 #if !defined(SOL_TCP)
3148 #define SOL_TCP IPPROTO_TCP
3149 #endif
3150 setsockopt(FD(c), SOL_TCP, TCP_NODELAY, (char *) &on, sizeof(on));
3151 #if defined(TCP_QUICKACK)
>>> CID 340736: (CHECKED_RETURN)
>>> Calling "setsockopt((SOCKET)(size_t)c->fd, 6, 12, (char *)&on, 4U)" without checking return value. This library function may fail and return an error code.
3152 setsockopt(FD(c), SOL_TCP, TCP_QUICKACK, (char *) &on, sizeof(on));
3153 #endif
3154 setsockopt(FD(c), SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on));
3155 #if ESP32 || ESP8266 || defined(__linux__)
3156 int idle = 60;
3157 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle));
/mongoose/mongoose.c: 3150 in setsockopts()
3144 (void) c;
3145 #else
3146 int on = 1;
3147 #if !defined(SOL_TCP)
3148 #define SOL_TCP IPPROTO_TCP
3149 #endif
>>> CID 340736: (CHECKED_RETURN)
>>> Calling "setsockopt((SOCKET)(size_t)c->fd, 6, 1, (char *)&on, 4U)" without checking return value. This library function may fail and return an error code.
3150 setsockopt(FD(c), SOL_TCP, TCP_NODELAY, (char *) &on, sizeof(on));
3151 #if defined(TCP_QUICKACK)
3152 setsockopt(FD(c), SOL_TCP, TCP_QUICKACK, (char *) &on, sizeof(on));
3153 #endif
3154 setsockopt(FD(c), SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on));
3155 #if ESP32 || ESP8266 || defined(__linux__)
/mongoose/mongoose.c: 3157 in setsockopts()
3151 #if defined(TCP_QUICKACK)
3152 setsockopt(FD(c), SOL_TCP, TCP_QUICKACK, (char *) &on, sizeof(on));
3153 #endif
3154 setsockopt(FD(c), SOL_SOCKET, SO_KEEPALIVE, (char *) &on, sizeof(on));
3155 #if ESP32 || ESP8266 || defined(__linux__)
3156 int idle = 60;
>>> CID 340736: (CHECKED_RETURN)
>>> Calling "setsockopt((SOCKET)(size_t)c->fd, IPPROTO_TCP, 4, &idle, 4U)" without checking return value. This library function may fail and return an error code.
3157 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle));
3158 #endif
3159 #if !defined(_WIN32) && !defined(__QNX__)
3160 {
3161 int cnt = 3, intvl = 20;
3162 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPCNT, &cnt, sizeof(cnt));
/mongoose/mongoose.c: 3163 in setsockopts()
3157 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle));
3158 #endif
3159 #if !defined(_WIN32) && !defined(__QNX__)
3160 {
3161 int cnt = 3, intvl = 20;
3162 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPCNT, &cnt, sizeof(cnt));
>>> CID 340736: (CHECKED_RETURN)
>>> Calling "setsockopt((SOCKET)(size_t)c->fd, IPPROTO_TCP, 5, &intvl, 4U)" without checking return value. This library function may fail and return an error code.
3163 setsockopt(FD(c), IPPROTO_TCP, TCP_KEEPINTVL, &intvl, sizeof(intvl));
3164 }
3165 #endif
3166 #endif
3167 }
3168

** CID 340735: (CHECKED_RETURN)
/mongoose/mongoose.c: 4194 in mg_file_write()
/mongoose/mongoose.c: 4191 in mg_file_write()


________________________________________________________________________________________________________
*** CID 340735: (CHECKED_RETURN)
/mongoose/mongoose.c: 4194 in mg_file_write()
4188 result = fwrite(buf, 1, len, fp) == len;
4189 fclose(fp);
4190 if (result) {
4191 remove(path);
4192 rename(tmp, path);
4193 } else {
>>> CID 340735: (CHECKED_RETURN)
>>> Calling "remove(tmp)" without checking return value. This library function may fail and return an error code.
4194 remove(tmp);
4195 }
4196 }
4197 return result;
4198 }
4199
/mongoose/mongoose.c: 4191 in mg_file_write()
4185 snprintf(tmp, sizeof(tmp), "%s.%d", path, rand());
4186 fp = fopen(tmp, "wb");
4187 if (fp != NULL) {
4188 result = fwrite(buf, 1, len, fp) == len;
4189 fclose(fp);
4190 if (result) {
>>> CID 340735: (CHECKED_RETURN)
>>> Calling "remove(path)" without checking return value. This library function may fail and return an error code.
4191 remove(path);
4192 rename(tmp, path);
4193 } else {
4194 remove(tmp);
4195 }
4196 }

** CID 340734: (TAINTED_SCALAR)
/mongoose/mongoose.c: 222 in mg_dns_parse()
/mongoose/mongoose.c: 227 in mg_dns_parse()


________________________________________________________________________________________________________
*** CID 340734: (TAINTED_SCALAR)
/mongoose/mongoose.c: 222 in mg_dns_parse()
216
217 if (len < sizeof(*h)) return 0; // Too small, headers dont fit
218 if (mg_ntohs(h->num_questions) > 1) return 0; // Sanity
219 if (mg_ntohs(h->num_answers) > 10) return 0; // Sanity
220 dm->txnid = mg_ntohs(h->txnid);
221
>>> CID 340734: (TAINTED_SCALAR)
>>> Using tainted variable "mg_ntohs(h->num_questions)" as a loop boundary.
222 for (i = 0; i < mg_ntohs(h->num_questions); i++) {
223 if ((n = mg_dns_parse_rr(buf, len, ofs, true, &rr)) == 0) return false;
224 // LOG(LL_INFO, ("Q %zu %zu", ofs, n));
225 ofs += n;
226 }
227 for (i = 0; i < mg_ntohs(h->num_answers); i++) {
/mongoose/mongoose.c: 227 in mg_dns_parse()
221
222 for (i = 0; i < mg_ntohs(h->num_questions); i++) {
223 if ((n = mg_dns_parse_rr(buf, len, ofs, true, &rr)) == 0) return false;
224 // LOG(LL_INFO, ("Q %zu %zu", ofs, n));
225 ofs += n;
226 }
>>> CID 340734: (TAINTED_SCALAR)
>>> Using tainted variable "mg_ntohs(h->num_answers)" as a loop boundary.
227 for (i = 0; i < mg_ntohs(h->num_answers); i++) {
228 // LOG(LL_INFO, ("A -- %zu %zu %s", ofs, n, dm->name));
229 if ((n = mg_dns_parse_rr(buf, len, ofs, false, &rr)) == 0) return false;
230 mg_dns_parse_name(buf, len, ofs, dm->name, sizeof(dm->name));
231 ofs += n;
232

** CID 340733: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 700 in p_seek()


________________________________________________________________________________________________________
*** CID 340733: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 700 in p_seek()
694 return fwrite(buf, 1, len, (FILE *) fp);
695 }
696
697 static size_t p_seek(void *fp, size_t offset) {
698 #if _FILE_OFFSET_BITS == 64 || _POSIX_C_SOURCE >= 200112L || \
699 _XOPEN_SOURCE >= 600
>>> CID 340733: Error handling issues (CHECKED_RETURN)
>>> Calling "fseeko((FILE *)fp, (off_t)offset, 0)" without checking return value. This library function may fail and return an error code.
700 fseeko((FILE *) fp, (off_t) offset, SEEK_SET);
701 #else
702 fseek((FILE *) fp, (long) offset, SEEK_SET);
703 #endif
704 return (size_t) ftell((FILE *) fp);
705 }

** CID 340732: Resource leaks (RESOURCE_LEAK)
/suricatta/server_hawkbit.c: 2096 in server_status_ipc()


________________________________________________________________________________________________________
*** CID 340732: Resource leaks (RESOURCE_LEAK)
/suricatta/server_hawkbit.c: 2096 in server_status_ipc()
2090 struct timeval tv = {
2091 .tv_sec = server_hawkbit.server_status_time,
2092 .tv_usec = 0
2093 };
2094
2095 pthread_mutex_lock(&ipc_lock);
>>> CID 340732: Resource leaks (RESOURCE_LEAK)
>>> Failing to save or free storage allocated by "swupdate_time_iso8601(&tv)" leaks it.
2096 sprintf(msg->data.procmsg.buf,
2097 "{\"server\":{\"status\":%d,\"time\":\"%s\"}}",
2098 server_hawkbit.server_status,
2099 swupdate_time_iso8601(&tv));
2100 msg->data.procmsg.len = strlen(msg->data.procmsg.buf);
2101 pthread_mutex_unlock(&ipc_lock);

** CID 340731: Insecure data handling (TAINTED_SCALAR)
/mongoose/mongoose.c: 2263 in mg_mqtt_next_topic()


________________________________________________________________________________________________________
*** CID 340731: Insecure data handling (TAINTED_SCALAR)
/mongoose/mongoose.c: 2263 in mg_mqtt_next_topic()
2257 if (pos >= msg->dgram.len) return 0;
2258
2259 topic->len = (size_t) (((unsigned) buf[0]) << 8 | buf[1]);
2260 topic->ptr = (char *) buf + 2;
2261 new_pos = pos + 2 + topic->len + (qos == NULL ? 0 : 1);
2262 if ((size_t) new_pos > msg->dgram.len) return 0;
>>> CID 340731: Insecure data handling (TAINTED_SCALAR)
>>> Using tainted variable "2UL + topic->len" as an index to pointer "buf".
2263 if (qos != NULL) *qos = buf[2 + topic->len];
2264 return new_pos;
2265 }
2266
2267 size_t mg_mqtt_next_sub(struct mg_mqtt_message *msg, struct mg_str *topic,
2268 uint8_t *qos, size_t pos) {

** CID 340730: Error handling issues (NEGATIVE_RETURNS)
/mongoose/mongoose.c: 4168 in mg_file_read()


________________________________________________________________________________________________________
*** CID 340730: Error handling issues (NEGATIVE_RETURNS)
/mongoose/mongoose.c: 4168 in mg_file_read()
4162 if ((fp = fopen(path, "rb")) != NULL) {
4163 fseek(fp, 0, SEEK_END);
4164 size = (size_t) ftell(fp);
4165 rewind(fp);
4166 data = (char *) calloc(1, size + 1);
4167 if (data != NULL) {
>>> CID 340730: Error handling issues (NEGATIVE_RETURNS)
>>> "size" is passed to a parameter that cannot be negative.
4168 if (fread(data, 1, size, fp) != size) {
4169 free(data);
4170 data = NULL;
4171 } else {
4172 data[size] = '\0';
4173 if (sizep != NULL) *sizep = size;

** CID 340729: (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 340729: (TAINTED_SCALAR)
/mongoose/mongoose.c: 4680 in mg_ws_cb()
4674 switch (op) {
4675 case WEBSOCKET_OP_CONTINUE:
4676 mg_call(c, MG_EV_WS_CTL, &m);
4677 break;
4678 case WEBSOCKET_OP_PING:
4679 LOG(LL_DEBUG, ("%s", "WS PONG"));
>>> CID 340729: (TAINTED_SCALAR)
>>> Passing tainted expression "msg.data_len" to "mg_ws_send", which uses it as an offset.
4680 mg_ws_send(c, s, msg.data_len, WEBSOCKET_OP_PONG);
4681 mg_call(c, MG_EV_WS_CTL, &m);
4682 break;
4683 case WEBSOCKET_OP_PONG:
4684 mg_call(c, MG_EV_WS_CTL, &m);
4685 break;
/mongoose/mongoose.c: 4711 in mg_ws_cb()
4705 len -= msg.header_len;
4706 ofs += len;
4707 c->pfn_data = (void *) ofs;
4708 // LOG(LL_INFO, ("FRAG %d [%.*s]", (int) ofs, (int) ofs, c->recv.buf));
4709 }
4710 // Remove non-fragmented frame
>>> CID 340729: (TAINTED_SCALAR)
>>> Passing tainted expression "c->recv.len" to "mg_iobuf_del", which uses it as an offset.
4711 if (final && op) mg_iobuf_del(&c->recv, ofs, len);
4712 // Last chunk of the fragmented frame
4713 if (final && !op) {
4714 m.flags = c->recv.buf[0];
4715 m.data = mg_str_n((char *) &c->recv.buf[1], (size_t) (ofs - 1));
4716 mg_call(c, MG_EV_WS_MSG, &m);
/mongoose/mongoose.c: 4680 in mg_ws_cb()
4674 switch (op) {
4675 case WEBSOCKET_OP_CONTINUE:
4676 mg_call(c, MG_EV_WS_CTL, &m);
4677 break;
4678 case WEBSOCKET_OP_PING:
4679 LOG(LL_DEBUG, ("%s", "WS PONG"));
>>> CID 340729: (TAINTED_SCALAR)
>>> Passing tainted expression "msg.data_len" to "mg_ws_send", which uses it as an offset.
4680 mg_ws_send(c, s, msg.data_len, WEBSOCKET_OP_PONG);
4681 mg_call(c, MG_EV_WS_CTL, &m);
4682 break;
4683 case WEBSOCKET_OP_PONG:
4684 mg_call(c, MG_EV_WS_CTL, &m);
4685 break;
/mongoose/mongoose.c: 4704 in mg_ws_cb()
4698 break;
4699 }
4700
4701 // Handle fragmented frames: strip header, keep in c->recv
4702 if (final == 0 || op == 0) {
4703 if (op) ofs++, len--, msg.header_len--; // First frame
>>> CID 340729: (TAINTED_SCALAR)
>>> Passing tainted expression "c->recv.len" to "mg_iobuf_del", which uses it as an offset.
4704 mg_iobuf_del(&c->recv, ofs, msg.header_len); // Strip header
4705 len -= msg.header_len;
4706 ofs += len;
4707 c->pfn_data = (void *) ofs;
4708 // LOG(LL_INFO, ("FRAG %d [%.*s]", (int) ofs, (int) ofs, c->recv.buf));
4709 }

** CID 340728: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 3279 in mg_mgr_wakeup()


________________________________________________________________________________________________________
*** CID 340728: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 3279 in mg_mgr_wakeup()
3273
3274 return result;
3275 }
3276
3277 void mg_mgr_wakeup(struct mg_connection *c) {
3278 LOG(LL_INFO, ("skt: %p", c->pfn_data));
>>> CID 340728: Error handling issues (CHECKED_RETURN)
>>> Calling "send((SOCKET)(size_t)c->pfn_data, "\1", 1UL, 0)" without checking return value. This library function may fail and return an error code.
3279 send((SOCKET) (size_t) c->pfn_data, "\x01", 1, MSG_NONBLOCKING);
3280 }
3281
3282 static void pf1(struct mg_connection *c, int ev, void *ev_data, void *fn_data) {
3283 if (ev == MG_EV_READ) mg_iobuf_free(&c->recv);
3284 (void) ev_data, (void) fn_data;

** CID 340727: (NULL_RETURNS)


________________________________________________________________________________________________________
*** CID 340727: (NULL_RETURNS)
/mongoose/mongoose_multipart.c: 353 in multipart_upload_handler()
347 if (s->len >= 9 && strncmp(s->ptr, "multipart", 9) == 0) {
348 /* New request - new proto data */
349 nc->label[0] = 'M';
350
351 nc->pfn = fn;
352 nc->pfn_data = calloc(1, sizeof(struct mg_http_multipart_stream));
>>> CID 340727: (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "nc->pfn_data" when calling "mg_http_multipart_begin".
353 mg_http_multipart_begin(nc, hm);
354 mg_http_multipart_continue(nc);
355 return;
356 }
357 }
/mongoose/mongoose_multipart.c: 354 in multipart_upload_handler()
348 /* New request - new proto data */
349 nc->label[0] = 'M';
350
351 nc->pfn = fn;
352 nc->pfn_data = calloc(1, sizeof(struct mg_http_multipart_stream));
353 mg_http_multipart_begin(nc, hm);
>>> CID 340727: (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "nc->pfn_data" when calling "mg_http_multipart_continue".
354 mg_http_multipart_continue(nc);
355 return;
356 }
357 }

** CID 340726: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 680 in p_open()


________________________________________________________________________________________________________
*** CID 340726: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 680 in p_open()
674 fp = (void *) _wfopen(b1, b2);
675 #else
676 fp = (void *) fopen(path, mode);
677 #endif
678 if (fp == NULL) return NULL;
679 fd = (struct mg_fd *) calloc(1, sizeof(*fd));
>>> CID 340726: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing "fd", which is known to be "NULL".
680 fd->fd = fp;
681 fd->fs = &mg_fs_posix;
682 return fd;
683 }
684
685 static void p_close(struct mg_fd *fd) {

** CID 340725: Security best practices violations (DC.WEAK_CRYPTO)
/mongoose/mongoose.c: 4230 in mg_random()


________________________________________________________________________________________________________
*** CID 340725: Security best practices violations (DC.WEAK_CRYPTO)
/mongoose/mongoose.c: 4230 in mg_random()
4224 if (fread(buf, 1, len, fp) == len) done = true;
4225 fclose(fp);
4226 }
4227 #endif
4228 // Fallback to a pseudo random gen
4229 if (!done) {
>>> CID 340725: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
4230 while (len--) *p++ = (unsigned char) (rand() & 255);
4231 }
4232 }
4233 #endif
4234
4235 bool mg_globmatch(const char *s1, size_t n1, const char *s2, size_t n2) {

** CID 340724: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose_multipart.c: 347 in multipart_upload_handler()


________________________________________________________________________________________________________
*** CID 340724: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose_multipart.c: 347 in multipart_upload_handler()
341 return;
342 }
343 }
344
345 if (hm->chunk.len >= 0 && ev == MG_EV_HTTP_CHUNK) {
346 s = mg_http_get_header(hm, "Content-Type");
>>> CID 340724: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing "s", which is known to be "NULL".
347 if (s->len >= 9 && strncmp(s->ptr, "multipart", 9) == 0) {
348 /* New request - new proto data */
349 nc->label[0] = 'M';
350
351 nc->pfn = fn;
352 nc->pfn_data = calloc(1, sizeof(struct mg_http_multipart_stream));

** CID 340723: Security best practices violations (DC.WEAK_CRYPTO)
/mongoose/mongoose.c: 4185 in mg_file_write()


________________________________________________________________________________________________________
*** CID 340723: Security best practices violations (DC.WEAK_CRYPTO)
/mongoose/mongoose.c: 4185 in mg_file_write()
4179 }
4180
4181 bool mg_file_write(const char *path, const void *buf, size_t len) {
4182 bool result = false;
4183 FILE *fp;
4184 char tmp[MG_PATH_MAX];
>>> CID 340723: Security best practices violations (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
4185 snprintf(tmp, sizeof(tmp), "%s.%d", path, rand());
4186 fp = fopen(tmp, "wb");
4187 if (fp != NULL) {
4188 result = fwrite(buf, 1, len, fp) == len;
4189 fclose(fp);
4190 if (result) {

** CID 340722: Control flow issues (NO_EFFECT)
/mongoose/mongoose_multipart.c: 345 in multipart_upload_handler()


________________________________________________________________________________________________________
*** CID 340722: Control flow issues (NO_EFFECT)
/mongoose/mongoose_multipart.c: 345 in multipart_upload_handler()
339 mg_call(nc, MG_EV_HTTP_MULTIPART_REQUEST_END, &mp);
340 mp_stream->state = MPS_FINISHED;
341 return;
342 }
343 }
344
>>> CID 340722: Control flow issues (NO_EFFECT)
>>> This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "hm->chunk.len >= 0UL".
345 if (hm->chunk.len >= 0 && ev == MG_EV_HTTP_CHUNK) {
346 s = mg_http_get_header(hm, "Content-Type");
347 if (s->len >= 9 && strncmp(s->ptr, "multipart", 9) == 0) {
348 /* New request - new proto data */
349 nc->label[0] = 'M';
350

** CID 340721: (OVERRUN)
/mongoose/mongoose.c: 4301 in mg_hexdump()
/mongoose/mongoose.c: 4301 in mg_hexdump()


________________________________________________________________________________________________________
*** CID 340721: (OVERRUN)
/mongoose/mongoose.c: 4301 in mg_hexdump()
4295 if (i > 0 && dlen > n)
4296 n += (size_t) snprintf(dst + n, dlen - n, " %s\n", ascii);
4297 if (dlen > n)
4298 n += (size_t) snprintf(dst + n, dlen - n, "%04x ", (int) (i + ofs));
4299 }
4300 if (dlen < n) break;
>>> CID 340721: (OVERRUN)
>>> Overrunning dynamic array "dst" at offset corresponding to index variable "n" through dereference in call to "snprintf".
4301 n += (size_t) snprintf(dst + n, dlen - n, " %02x", p[i]);
4302 ascii[idx] = (char) (p[i] < 0x20 || p[i] > 0x7e ? '.' : p[i]);
4303 ascii[idx + 1] = '\0';
4304 }
4305 while (i++ % 16) {
4306 if (n < dlen) n += (size_t) snprintf(dst + n, dlen - n, "%s", " ");
/mongoose/mongoose.c: 4301 in mg_hexdump()
4295 if (i > 0 && dlen > n)
4296 n += (size_t) snprintf(dst + n, dlen - n, " %s\n", ascii);
4297 if (dlen > n)
4298 n += (size_t) snprintf(dst + n, dlen - n, "%04x ", (int) (i + ofs));
4299 }
4300 if (dlen < n) break;
>>> CID 340721: (OVERRUN)
>>> Overrunning dynamic array "dst" at offset corresponding to index variable "n" through dereference in call to "snprintf".
4301 n += (size_t) snprintf(dst + n, dlen - n, " %02x", p[i]);
4302 ascii[idx] = (char) (p[i] < 0x20 || p[i] > 0x7e ? '.' : p[i]);
4303 ascii[idx + 1] = '\0';
4304 }
4305 while (i++ % 16) {
4306 if (n < dlen) n += (size_t) snprintf(dst + n, dlen - n, "%s", " ");


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrffGusdB2gY411RFCZULj23zVH-2FYjULe-2FZVatHaTNOtXGWK7d76ThnungrCH2R2Ak-3DEGc2_U-2BcIo0IeFG9EIIQe7AiEg4oWkKw9GoOEX2k5mfx97s8hkRJNOiwyrjcEEzbe4O-2FG7jXnIf5F5MkyL1PsCWb44XINgJfBkW690OvPN3afsFVK9Wa8Qxs2Bk0uYAU-2F0M-2FlXLSSVpYDoR-2FIozje6-2F4Wy4FJUfH-2B3yJ1cpFfYqS7KMQMPWS8wFauLWWR2EcAD8OGgIbvOh3JpnyvUrDVxkqEHXHYUUrZZJpBL3x3QbDriA4-3D

scan-...@coverity.com

unread,
Oct 29, 2021, 8:15:50 AM10/29/21
to swup...@googlegroups.com
Hi,

Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.

20 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.
16 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 20 of 20 defect(s)


** CID 318473: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 11545 in mg_send_dns_query()


________________________________________________________________________________________________________
*** CID 318473: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 11545 in mg_send_dns_query()
11539
11540 void mg_send_dns_query(struct mg_connection *nc, const char *name,
11541 int query_type) {
11542 struct mg_dns_message *msg =
11543 (struct mg_dns_message *) MG_CALLOC(1, sizeof(*msg));
11544 struct mbuf pkt;
>>> CID 318473: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing "msg", which is known to be "NULL".
11545 struct mg_dns_resource_record *rr = &msg->questions[0];
11546
11547 DBG(("%s %d", name, query_type));
11548
11549 mbuf_init(&pkt, 64 /* Start small, it'll grow as needed. */);
11550

** CID 318404: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 3482 in mg_if_create_iface()


________________________________________________________________________________________________________
*** CID 318404: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 3482 in mg_if_create_iface()
3476
3477 int mg_num_ifaces = (int) (sizeof(mg_ifaces) / sizeof(mg_ifaces[0]));
3478
3479 struct mg_iface *mg_if_create_iface(const struct mg_iface_vtable *vtable,
3480 struct mg_mgr *mgr) {
3481 struct mg_iface *iface = (struct mg_iface *) MG_CALLOC(1, sizeof(*iface));
>>> CID 318404: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing "iface", which is known to be "NULL".
3482 iface->mgr = mgr;
3483 iface->data = NULL;
3484 iface->vtable = vtable;
3485 return iface;
3486 }
3487

** CID 316450: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 7178 in mg_http_serve_file_internal()


________________________________________________________________________________________________________
*** CID 316450: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 7178 in mg_http_serve_file_internal()
7172 snprintf(range, sizeof(range),
7173 "Content-Range: bytes %" INT64_FMT "-%" INT64_FMT
7174 "/%" INT64_FMT "\r\n",
7175 r1, r1 + cl - 1, (int64_t) st.st_size);
7176 #if _FILE_OFFSET_BITS == 64 || _POSIX_C_SOURCE >= 200112L || \
7177 _XOPEN_SOURCE >= 600
>>> CID 316450: Error handling issues (CHECKED_RETURN)
>>> Calling "fseeko(pd->file.fp, r1, 0)" without checking return value. This library function may fail and return an error code.
7178 fseeko(pd->file.fp, r1, SEEK_SET);
7179 #else
7180 fseek(pd->file.fp, (long) r1, SEEK_SET);
7181 #endif
7182 }
7183 }

** CID 314959: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 8474 in mg_file_upload_handler()


________________________________________________________________________________________________________
*** CID 314959: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 8474 in mg_file_upload_handler()
8468 fus = (struct file_upload_state *) MG_CALLOC(1, sizeof(*fus));
8469 if (fus == NULL) {
8470 nc->flags |= MG_F_CLOSE_IMMEDIATELY;
8471 return;
8472 }
8473 fus->lfn = (char *) MG_MALLOC(lfn.len + 1);
>>> CID 314959: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "fus->lfn" when calling "memcpy".
8474 memcpy(fus->lfn, lfn.p, lfn.len);
8475 fus->lfn[lfn.len] = '\0';
8476 if (lfn.p != mp->file_name) MG_FREE((char *) lfn.p);
8477 LOG(LL_DEBUG,
8478 ("%p Receiving file %s -> %s", nc, mp->file_name, fus->lfn));
8479 fus->fp = mg_fopen(fus->lfn, "wb");

** CID 314957: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 2452 in mg_mgr_init_opt()


________________________________________________________________________________________________________
*** CID 314957: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 2452 in mg_mgr_init_opt()
2446 opts.ifaces[MG_MAIN_IFACE] = opts.main_iface;
2447 }
2448 m->num_ifaces = opts.num_ifaces;
2449 m->ifaces =
2450 (struct mg_iface **) MG_MALLOC(sizeof(*m->ifaces) * opts.num_ifaces);
2451 for (i = 0; i < opts.num_ifaces; i++) {
>>> CID 314957: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing "m->ifaces", which is known to be "NULL".
2452 m->ifaces[i] = mg_if_create_iface(opts.ifaces[i], m);
2453 m->ifaces[i]->vtable->init(m->ifaces[i]);
2454 }
2455 }
2456 if (opts.nameserver != NULL) {
2457 m->nameserver = strdup(opts.nameserver);

** CID 314956: Null pointer dereferences (NULL_RETURNS)


________________________________________________________________________________________________________
*** CID 314956: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 12007 in mg_resolve_async_eh()
12001 req->last_time = now;
12002 req->retries++;
12003 }
12004 break;
12005 case MG_EV_RECV:
12006 msg = (struct mg_dns_message *) MG_MALLOC(sizeof(*msg));
>>> CID 314956: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "msg" when calling "mg_parse_dns".
12007 if (mg_parse_dns(nc->recv_mbuf.buf, *(int *) data, msg) == 0 &&
12008 msg->num_answers > 0) {
12009 req->callback(msg, req->data, MG_RESOLVE_OK);
12010 nc->user_data = NULL;
12011 MG_FREE(req);
12012 } else {

** CID 314954: Null pointer dereferences (NULL_RETURNS)


________________________________________________________________________________________________________
*** CID 314954: Null pointer dereferences (NULL_RETURNS)
/mongoose/mongoose.c: 7464 in mg_parse_http_basic_auth()
7458 char fmt[64];
7459 int res = 0;
7460
7461 if (mg_strncmp(*hdr, mg_mk_str("Basic "), 6) != 0) return -1;
7462
7463 buf = (char *) MG_MALLOC(hdr->len);
>>> CID 314954: Null pointer dereferences (NULL_RETURNS)
>>> Dereferencing a pointer that might be "NULL" "buf" when calling "cs_base64_decode".
7464 cs_base64_decode((unsigned char *) hdr->p + 6, hdr->len, buf, NULL);
7465
7466 /* e.g. "%123[^:]:%321[^\n]" */
7467 snprintf(fmt, sizeof(fmt), "%%%" SIZE_T_FMT "[^:]:%%%" SIZE_T_FMT "[^\n]",
7468 user_len - 1, pass_len - 1);
7469 if (sscanf(buf, fmt, user, pass) == 0) {

** CID 292130: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 8523 in mg_file_upload_handler()


________________________________________________________________________________________________________
*** CID 292130: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 8523 in mg_file_upload_handler()
8517 "Content-Type: text/plain\r\n"
8518 "Connection: close\r\n\r\n");
8519 mg_printf(nc, "Failed to write to %s: %d, wrote %d", mp->file_name,
8520 mg_get_errno(), (int) fus->num_recd);
8521 }
8522 fclose(fus->fp);
>>> CID 292130: Error handling issues (CHECKED_RETURN)
>>> Calling "remove(fus->lfn)" without checking return value. This library function may fail and return an error code.
8523 remove(fus->lfn);
8524 fus->fp = NULL;
8525 /* Do not close the connection just yet, discard remainder of the data.
8526 * This is because at the time of writing some browsers (Chrome) fail to
8527 * render response before all the data is sent. */
8528 return;

** CID 292127: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 10397 in mg_start_thread()


________________________________________________________________________________________________________
*** CID 292127: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 10397 in mg_start_thread()
10391 (void) pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
10392
10393 #if defined(MG_STACK_SIZE) && MG_STACK_SIZE > 1
10394 (void) pthread_attr_setstacksize(&attr, MG_STACK_SIZE);
10395 #endif
10396
>>> CID 292127: Error handling issues (CHECKED_RETURN)
>>> Calling "pthread_create" without checking return value (as is done elsewhere 5 out of 6 times).
10397 pthread_create(&thread_id, &attr, f, p);
10398 pthread_attr_destroy(&attr);
10399
10400 return (void *) thread_id;
10401 #endif
10402 }

** CID 183765: Null pointer dereferences (FORWARD_NULL)
/mongoose/mongoose.c: 7388 in mg_http_parse_header_internal()


________________________________________________________________________________________________________
*** CID 183765: Null pointer dereferences (FORWARD_NULL)
/mongoose/mongoose.c: 7388 in mg_http_parse_header_internal()
7382 const char *var_name,
7383 struct altbuf *ab) {
7384 int ch = ' ', ch1 = ',', ch2 = ';', n = strlen(var_name);
7385 const char *p, *end = hdr ? hdr->p + hdr->len : NULL, *s = NULL;
7386
7387 /* Find where variable starts */
>>> CID 183765: Null pointer dereferences (FORWARD_NULL)
>>> Dereferencing null pointer "hdr".
7388 for (s = hdr->p; s != NULL && s + n < end; s++) {
7389 if ((s == hdr->p || s[-1] == ch || s[-1] == ch1 || s[-1] == ';') &&
7390 s[n] == '=' && !strncmp(s, var_name, n))
7391 break;
7392 }
7393

** CID 183763: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/mongoose/mongoose.c: 894 in cs_timegm()


________________________________________________________________________________________________________
*** CID 183763: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
/mongoose/mongoose.c: 894 in cs_timegm()
888 60 *
889 (tm->tm_min /* Minute = 60 seconds */
890 +
891 60 * (tm->tm_hour /* Hour = 60 minutes */
892 +
893 24 * (month_day[month] + tm->tm_mday - 1 /* Day = 24 hours */
>>> CID 183763: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
>>> Potentially overflowing expression "365 * (year - 70)" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "int64_t" (64 bits, signed).
894 + 365 * (year - 70) /* Year = 365 days */
895 + (year_for_leap - 69) / 4 /* Every 4 years is leap... */
896 - (year_for_leap - 1) / 100 /* Except centuries... */
897 + (year_for_leap + 299) / 400))); /* Except 400s. */
898 return rt < 0 ? -1 : (double) rt;
899 }

** CID 183761: Null pointer dereferences (FORWARD_NULL)
/mongoose/mongoose.c: 10977 in mg_send_mqtt_handshake_opt()


________________________________________________________________________________________________________
*** CID 183761: Null pointer dereferences (FORWARD_NULL)
/mongoose/mongoose.c: 10977 in mg_send_mqtt_handshake_opt()
10971 }
10972
10973 if (opts.flags & MG_MQTT_HAS_WILL) {
10974 total_len += 2 + wt_len + 2 + wm_len;
10975 }
10976 if (opts.flags & MG_MQTT_HAS_USER_NAME) {
>>> CID 183761: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "opts.user_name" to "strlen", which dereferences it.
10977 user_len = strlen(opts.user_name);
10978 total_len += 2 + user_len;
10979 }
10980 if (opts.flags & MG_MQTT_HAS_PASSWORD) {
10981 pw_len = strlen(opts.password);
10982 total_len += 2 + pw_len;

** CID 183759: Insecure data handling (TAINTED_SCALAR)
/mongoose/mongoose.c: 11084 in mg_mqtt_next_subscribe_topic()


________________________________________________________________________________________________________
*** CID 183759: Insecure data handling (TAINTED_SCALAR)
/mongoose/mongoose.c: 11084 in mg_mqtt_next_subscribe_topic()
11078 if ((size_t) pos >= msg->payload.len) return -1;
11079
11080 topic->len = buf[0] << 8 | buf[1];
11081 topic->p = (char *) buf + 2;
11082 new_pos = pos + 2 + topic->len + 1;
11083 if ((size_t) new_pos > msg->payload.len) return -1;
>>> CID 183759: Insecure data handling (TAINTED_SCALAR)
>>> Using tainted variable "2UL + topic->len" as an index to pointer "buf".
11084 *qos = buf[2 + topic->len];
11085 return new_pos;
11086 }
11087
11088 void mg_mqtt_unsubscribe(struct mg_connection *nc, char **topics,
11089 size_t topics_len, uint16_t message_id) {

** CID 183757: Null pointer dereferences (FORWARD_NULL)
/mongoose/mongoose.c: 10981 in mg_send_mqtt_handshake_opt()


________________________________________________________________________________________________________
*** CID 183757: Null pointer dereferences (FORWARD_NULL)
/mongoose/mongoose.c: 10981 in mg_send_mqtt_handshake_opt()
10975 }
10976 if (opts.flags & MG_MQTT_HAS_USER_NAME) {
10977 user_len = strlen(opts.user_name);
10978 total_len += 2 + user_len;
10979 }
10980 if (opts.flags & MG_MQTT_HAS_PASSWORD) {
>>> CID 183757: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "opts.password" to "strlen", which dereferences it.
10981 pw_len = strlen(opts.password);
10982 total_len += 2 + pw_len;
10983 }
10984
10985 mg_send_mqtt_header(nc, MG_MQTT_CMD_CONNECT, 0, total_len);
10986 mg_send(nc, "\00\04MQTT\04", 7);

** CID 183756: Integer handling issues (NEGATIVE_RETURNS)


________________________________________________________________________________________________________
*** CID 183756: Integer handling issues (NEGATIVE_RETURNS)
/mongoose/mongoose.c: 7068 in mg_http_send_redirect()
7062 "Cache-Control: no-cache\r\n"
7063 "%.*s%s",
7064 (int) location.len, location.p, bl, (int) extra_headers.len,
7065 extra_headers.p, (extra_headers.len > 0 ? "\r\n" : ""));
7066 mg_send_response_line(nc, status_code, phead);
7067 if (phead != bhead) MG_FREE(phead);
>>> CID 183756: Integer handling issues (NEGATIVE_RETURNS)
>>> "bl" is passed to a parameter that cannot be negative.
7068 mg_send(nc, pbody, bl);
7069 if (pbody != bbody) MG_FREE(pbody);
7070 }
7071
7072 void mg_send_head(struct mg_connection *c, int status_code,
7073 int64_t content_length, const char *extra_headers) {

** CID 166498: (DC.WEAK_CRYPTO)
/mongoose/mongoose.c: 10028 in mg_ws_random_mask()
/mongoose/mongoose.c: 10026 in mg_ws_random_mask()


________________________________________________________________________________________________________
*** CID 166498: (DC.WEAK_CRYPTO)
/mongoose/mongoose.c: 10028 in mg_ws_random_mask()
10022 #if MG_DISABLE_WS_RANDOM_MASK
10023 mask = 0xefbeadde; /* generated with a random number generator, I swear */
10024 #else
10025 if (sizeof(long) >= 4) {
10026 mask = (uint32_t) rand();
10027 } else if (sizeof(long) == 2) {
>>> CID 166498: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
10028 mask = (uint32_t) rand() << 16 | (uint32_t) rand();
10029 }
10030 #endif
10031 return mask;
10032 }
10033
/mongoose/mongoose.c: 10026 in mg_ws_random_mask()
10020 * that lacks rand().
10021 */
10022 #if MG_DISABLE_WS_RANDOM_MASK
10023 mask = 0xefbeadde; /* generated with a random number generator, I swear */
10024 #else
10025 if (sizeof(long) >= 4) {
>>> CID 166498: (DC.WEAK_CRYPTO)
>>> "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
10026 mask = (uint32_t) rand();
10027 } else if (sizeof(long) == 2) {
10028 mask = (uint32_t) rand() << 16 | (uint32_t) rand();
10029 }
10030 #endif
10031 return mask;

** CID 166497: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 9029 in mg_start_process()


________________________________________________________________________________________________________
*** CID 166497: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 9029 in mg_start_process()
9023 }
9024 snprintf(buf, sizeof(buf),
9025 "Status: 500\r\n\r\n"
9026 "500 Server Error: %s%s%s: %s",
9027 interp == NULL ? "" : interp, interp == NULL ? "" : " ", cmd,
9028 strerror(errno));
>>> CID 166497: Error handling issues (CHECKED_RETURN)
>>> Calling "send(1, buf, strlen(buf), 0)" without checking return value. This library function may fail and return an error code.
9029 send(1, buf, strlen(buf), 0);
9030 _exit(EXIT_FAILURE); /* exec call failed */
9031 }
9032
9033 return (pid != 0);
9034 }

** CID 166486: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 4179 in mg_sock_get_addr()


________________________________________________________________________________________________________
*** CID 166486: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 4179 in mg_sock_get_addr()
4173 union socket_address *sa) {
4174 socklen_t slen = sizeof(*sa);
4175 memset(sa, 0, slen);
4176 if (remote) {
4177 getpeername(sock, &sa->sa, &slen);
4178 } else {
>>> CID 166486: Error handling issues (CHECKED_RETURN)
>>> Calling "getsockname(sock, __SOCKADDR_ARG({.__sockaddr__ = &sa->sa}), &slen)" without checking return value. This library function may fail and return an error code.
4179 getsockname(sock, &sa->sa, &slen);
4180 }
4181 }
4182
4183 void mg_sock_to_str(sock_t sock, char *buf, size_t len, int flags) {
4184 union socket_address sa;

** CID 166468: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 10410 in mg_set_close_on_exec()


________________________________________________________________________________________________________
*** CID 166468: Error handling issues (CHECKED_RETURN)
/mongoose/mongoose.c: 10410 in mg_set_close_on_exec()
10404
10405 /* Set close-on-exec bit for a given socket. */
10406 void mg_set_close_on_exec(sock_t sock) {
10407 #if defined(_WIN32) && !defined(WINCE)
10408 (void) SetHandleInformation((HANDLE) sock, HANDLE_FLAG_INHERIT, 0);
10409 #elif defined(__unix__)
>>> CID 166468: Error handling issues (CHECKED_RETURN)
>>> Calling "fcntl(sock, 2, 1)" without checking return value. This library function may fail and return an error code.
10410 fcntl(sock, F_SETFD, FD_CLOEXEC);
10411 #else
10412 (void) sock;
10413 #endif
10414 }
10415

** CID 166433: Possible Control flow issues (DEADCODE)
/mongoose/mongoose.c: 8309 in mg_send_http_file()


________________________________________________________________________________________________________
*** CID 166433: Possible Control flow issues (DEADCODE)
/mongoose/mongoose.c: 8309 in mg_send_http_file()
8303 if (path_info->len > 0 && !is_cgi) {
8304 mg_http_send_error(nc, 501, NULL);
8305 MG_FREE(index_file);
8306 return;
8307 }
8308
>>> CID 166433: Possible Control flow issues (DEADCODE)
>>> Execution cannot reach the expression "opts->dav_document_root == NULL" inside this statement: "if (is_dav && opts->dav_doc...".
8309 if (is_dav && opts->dav_document_root == NULL) {
8310 mg_http_send_error(nc, 501, NULL);
8311 } else if (!mg_http_is_authorized(
8312 hm, mg_mk_str(path), opts->auth_domain, opts->global_auth_file,
8313 ((is_directory ? MG_AUTH_FLAG_IS_DIRECTORY : 0) |
8314 MG_AUTH_FLAG_IS_GLOBAL_PASS_FILE |


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrffGusdB2gY411RFCZULj23zVH-2FYjULe-2FZVatHaTNOtXGWK7d76ThnungrCH2R2Ak-3DJUpX_U-2BcIo0IeFG9EIIQe7AiEg4oWkKw9GoOEX2k5mfx97s9fELjUXLq9srRv8z9hWa-2FUNf9c9TWy-2BgcJ32WNi6qyQKRSsQY1jTKxiCtN94szyCLzdT-2BYdXFtESVCiicxKGXAW4GUhzTfY5sNr-2FFvidrYpSjfTbKuokqjrXe0Bj9CsCH-2F1bCeH2qrT4eKGutWW4nq2Wp5a8spd3FDSWhu15yvXolWH-2Fhe6V6qtgTZhUuARM0-3D

scan-...@coverity.com

unread,
Nov 2, 2021, 12:18:57 PM11/2/21
to swup...@googlegroups.com
Hi,

Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.

1 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 340916: Resource leaks (RESOURCE_LEAK)
/tools/swupdate-ipc.c: 463 in sysrestart()


________________________________________________________________________________________________________
*** CID 340916: Resource leaks (RESOURCE_LEAK)
/tools/swupdate-ipc.c: 463 in sysrestart()
457 long_options, NULL)) != EOF) {
458 switch (c) {
459 case 'w':
460 opt_w = 1;
461 break;
462 case 's':
>>> CID 340916: Resource leaks (RESOURCE_LEAK)
>>> Overwriting "SOCKET_PROGRESS_PATH" in "SOCKET_PROGRESS_PATH = strdup(optarg)" leaks the storage that "SOCKET_PROGRESS_PATH" points to.
463 SOCKET_PROGRESS_PATH = strdup(optarg);
464 break;
465 case 'h':
466 usage(argv[0]);
467 exit(0);
468 break;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrffGusdB2gY411RFCZULj23zVH-2FYjULe-2FZVatHaTNOtXGWK7d76ThnungrCH2R2Ak-3DJIVX_U-2BcIo0IeFG9EIIQe7AiEg4oWkKw9GoOEX2k5mfx97s-2Fr1DADiP0E5Fb7cnCc9G2h17W8PbzhYtBD2lC0o9rW0jks3Jgrf5kwhWpIH-2F1sqAUZVV-2BcMW1jcGtceWxtRg52t5DukRkLryP37is20MGspUYopOE2ZX5mzPVSevRAc-2FFus-2FA6zNrI0thybJJWPaPBvYavNpV4cLA0p8AqpqQLfWFHGox2bk8BygfDIG0TQkQ-3D

scan-...@coverity.com

unread,
Nov 2, 2021, 12:49:36 PM11/2/21
to swup...@googlegroups.com
Hi,

Please find the latest report on new defect(s) introduced to sbabic/swupdate found with Coverity Scan.

1 new defect(s) introduced to sbabic/swupdate found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 340917: Resource leaks (RESOURCE_LEAK)
/tools/swupdate-ipc.c: 464 in sysrestart()


________________________________________________________________________________________________________
*** CID 340917: Resource leaks (RESOURCE_LEAK)
/tools/swupdate-ipc.c: 464 in sysrestart()
458 long_options, NULL)) != EOF) {
459 switch (c) {
460 case 'w':
461 opt_w = 1;
462 break;
463 case 's':
>>> CID 340917: Resource leaks (RESOURCE_LEAK)
>>> Overwriting "socket_path" in "socket_path = strdup(optarg)" leaks the storage that "socket_path" points to.
464 socket_path = strdup(optarg);
465 break;
466 case 'h':
467 usage(argv[0]);
468 exit(0);
469 break;


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yrffGusdB2gY411RFCZULj23zVH-2FYjULe-2FZVatHaTNOtXGWK7d76ThnungrCH2R2Ak-3D5r5z_U-2BcIo0IeFG9EIIQe7AiEg4oWkKw9GoOEX2k5mfx97s9RCwufk20Su0Hz9VRrUeN9pMVkb36-2FV7m-2BVWj5ksobwUom2P7i8LzVh5EmMCe7pUB9e6zfM-2B1bxcgeLQkNj5CIs2-2F0dw63KTrPPiEodqcQCkixXI0L5o6P5VccCwNocxcOmPYEOuUQ87Tocw-2FuK-2Bj0t6IuU23WrZ2HRhi4x9EfQNgOJaZp-2F3RiOLdLws2Bb-2FE-3D

Reply all
Reply to author
Forward
0 new messages