{"action":"created","issue":{"url":"https://api.github.com/repos/AppImage/zsync2/issues/26","repository_url":"https://api.github.com/repos/AppImage/zsync2","labels_url":"https://api.github.com/repos/AppImage/zsync2/issues/26/labels{/name}","comments_url":"https://api.github.com/repos/AppImage/zsync2/issues/26/comments","events_url":"https://api.github.com/repos/AppImage/zsync2/issues/26/events","html_url":"https://github.com/AppImage/zsync2/issues/26","id":349723343,"node_id":"MDU6SXNzdWUzNDk3MjMzNDM=","number":26,"title":"Use consttime_memequal instead of memcmp.","user":{"login":"antony-jr","id":33930174,"node_id":"MDQ6VXNlcjMzOTMwMTc0","avatar_url":"https://avatars2.githubusercontent.com/u/33930174?v=4","gravatar_id":"","url":"https://api.github.com/users/antony-jr","html_url":"https://github.com/antony-jr","followers_url":"https://api.github.com/users/antony-jr/followers","following_url":"https://api.github.com/users/antony-jr/following{/other_user}","gists_url":"https://api.github.com/users/antony-jr/gists{/gist_id}","starred_url":"https://api.github.com/users/antony-jr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antony-jr/subscriptions","organizations_url":"https://api.github.com/users/antony-jr/orgs","repos_url":"https://api.github.com/users/antony-jr/repos","events_url":"https://api.github.com/users/antony-jr/events{/privacy}","received_events_url":"https://api.github.com/users/antony-jr/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-08-11T10:23:22Z","updated_at":"2018-08-11T15:57:00Z","closed_at":"2018-08-11T15:40:52Z","author_association":"NONE","body":"From the legacy source files , In ```lib/librcksum/rsum.c``` at https://github.com/AppImage/zsync2/blob/f93e10ab4d6204a8ec64ca5909fcc23417dbdcf7/lib/librcksum/rsum.c#L154 and https://github.com/AppImage/zsync2/blob/f93e10ab4d6204a8ec64ca5909fcc23417dbdcf7/lib/librcksum/rsum.c#L238, You use ```memcmp``` to compare security critical data(i.e cryptographic hash ) which is not recommended.\r\n\r\nFrom **memcmp** manual page (```$ man memcmp``` for more information) , The manual explicitly states not to use ```memcmp``` for comparing critical data.\r\n\r\n> Do not use memcmp() to compare security critical data, such as cryptographic secrets, because the required CPU time depends on the number of equal bytes.  Instead, a function that performs comparisons in constant time is required.  Some  operating systems  provide  such  a  function  (e.g., NetBSD's consttime_memequal()), but no such function is specified in POSIX.  On Linux, it may be necessary to implement such a function oneself.\r\n\r\nThus I think it would be nice if **zsync2** uses **consttime_memequal** , But it is only available in NetBSD , Therefore we have to write it ourselves , this is the function from NetBSD's source files.\r\n\r\n```\r\nint\r\nconsttime_memequal(const void *b1, const void *b2, size_t len)\r\n{\r\n const char *c1 = b1, *c2 = b2;\r\n int res = 0;\r\n\r\n while (len --)\r\n  res |= *c1++ ^ *c2++;\r\n\r\n /*\r\n  * If the compiler for your favourite architecture generates a\r\n  * conditional branch for `!res', it will be a data-dependent\r\n  * branch, in which case this should be replaced by\r\n  *\r\n  * return (1 - (1 & ((res - 1) >> 8)));\r\n  *\r\n  * or rewritten in assembly.\r\n  */\r\n return !res;\r\n}\r\n```\r\n\r\nThe above function can be declared as a **static** function in the source file(```rsum.c```) and can be used.\r\n\r\n**IMPORTANT**: *consttime_memequal() returns 0 if the given memory contents are distinct and 1 if its the same*.\r\n\r\n**consttime_memequal() manual:** https://www.daemon-systems.org/man/consttime_memequal.3.html"},"comment":{"url":"https://api.github.com/repos/AppImage/zsync2/issues/comments/412283971","html_url":"https://github.com/AppImage/zsync2/issues/26#issuecomment-412283971","issue_url":"https://api.github.com/repos/AppImage/zsync2/issues/26","id":412283971,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMjI4Mzk3MQ==","user":{"login":"antony-jr","id":33930174,"node_id":"MDQ6VXNlcjMzOTMwMTc0","avatar_url":"https://avatars2.githubusercontent.com/u/33930174?v=4","gravatar_id":"","url":"https://api.github.com/users/antony-jr","html_url":"https://github.com/antony-jr","followers_url":"https://api.github.com/users/antony-jr/followers","following_url":"https://api.github.com/users/antony-jr/following{/other_user}","gists_url":"https://api.github.com/users/antony-jr/gists{/gist_id}","starred_url":"https://api.github.com/users/antony-jr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/antony-jr/subscriptions","organizations_url":"https://api.github.com/users/antony-jr/orgs","repos_url":"https://api.github.com/users/antony-jr/repos","events_url":"https://api.github.com/users/antony-jr/events{/privacy}","received_events_url":"https://api.github.com/users/antony-jr/received_events","type":"User","site_admin":false},"created_at":"2018-08-11T15:57:00Z","updated_at":"2018-08-11T15:57:00Z","author_association":"NONE","body":"I know the difference between cryptographic secret and hash , I was just concerned about malicious data injection. Just to be safe , I thought using ```consttime_memequal``` would give us more security on such attacks. Sorry for the false alarm."}}
{"actor":{"display_login":"antony-jr"}}