After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 711063 - Assorted bug fixes
Assorted bug fixes
Status: RESOLVED FIXED
Product: libdmapsharing
Classification: Other
Component: General
git master
Other All
: Normal normal
: ---
Assigned To: W. Michael Petullo
W. Michael Petullo
Depends on:
Blocks:
 
 
Reported: 2013-10-29 10:45 UTC by Bastien Nocera
Modified: 2015-05-20 12:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use strlen() instead of hard-coding string length (897 bytes, patch)
2013-10-29 10:46 UTC, Bastien Nocera
committed Details | Review
Avoid OOB read with buggy servers (1.24 KB, patch)
2013-10-29 10:46 UTC, Bastien Nocera
committed Details | Review
Fix clang warning (1.21 KB, patch)
2013-10-29 10:46 UTC, Bastien Nocera
committed Details | Review
Fix incorrect fix for OOB reads with buggy servers (1.05 KB, patch)
2014-04-11 07:18 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2013-10-29 10:45:54 UTC
From a downstream check.
Comment 1 Bastien Nocera 2013-10-29 10:46:12 UTC
Created attachment 258425 [details] [review]
Use strlen() instead of hard-coding string length

This avoids hard to detect bugs when we want a different string length,
and will be optimised by the compiler anyway.
Comment 2 Bastien Nocera 2013-10-29 10:46:16 UTC
Created attachment 258426 [details] [review]
Avoid OOB read with buggy servers

If the server doesn't start the Content-Range field with "bytes="
we would have an out-of-bounds read trying to parse the content
of that field. Fall back to a 0 offset when a parsing error occurs.

See https://bugzilla.redhat.com/show_bug.cgi?id=1024020
Comment 3 Bastien Nocera 2013-10-29 10:46:20 UTC
Created attachment 258427 [details] [review]
Fix clang warning

dmap-md5.c:187:26: warning: 'memset' call operates on objects of type 'MD5_CTX'
while the size is based on a different
      type 'MD5_CTX *' [-Wsizeof-pointer-memaccess]
        memset (ctx, 0, sizeof (ctx));  /* In case it's sensitive */
                ~~~             ^~~

That should be "sizeof(*ctx)" instead.

See https://bugzilla.redhat.com/show_bug.cgi?id=1023528
Comment 4 W. Michael Petullo 2013-10-30 01:50:07 UTC
Fixed in Git master. May I have access to the Red Hat bugs?
Comment 5 Bastien Nocera 2013-11-05 09:31:09 UTC
(In reply to comment #4)
> Fixed in Git master. May I have access to the Red Hat bugs?

Done.
Comment 6 W. Michael Petullo 2013-11-07 14:12:08 UTC
I just released libdmapsharing 2.9.24, and this release includes the patches above.
Comment 7 Ray Strode [halfline] 2014-04-10 19:31:01 UTC
This failed QE checking on the downstream report
Comment 8 Ray Strode [halfline] 2014-04-10 19:31:54 UTC
Review of attachment 258426 [details] [review]:

::: libdmapsharing/daap-share.c
@@ +923,3 @@
 		gchar *content_range;
 
+		if (!g_ascii_strncasecmp (range_header, "bytes=", strlen("bytes="))) {

the check here is inverted since strcmp returns 0 not TRUE on match.  I'd recommend using g_str_has_prefix() instead.
Comment 9 Bastien Nocera 2014-04-11 07:18:07 UTC
Created attachment 274064 [details] [review]
Fix incorrect fix for OOB reads with buggy servers

3e347fd3e8e7e20afc562268f27fd3c2b79f4d0e tried to fix
problems with servers that had incorrect values in the
Content-Range field, except that the condition was reversed.

Correct g_ascii_strncasecmp() usage by using g_str_has_prefix()
instead.
Comment 10 W. Michael Petullo 2014-04-12 13:26:28 UTC
Fixed in libdmapsharing 2.9.25. Thank you.
Comment 11 Bastien Nocera 2015-05-20 12:14:14 UTC
Comment on attachment 274064 [details] [review]
Fix incorrect fix for OOB reads with buggy servers

Was committed in fbfd169b581d328df1b977dbcce6d2385bd271be