{"action":"created","issue":{"url":"https://api.github.com/repos/UnionOfRAD/lithium/issues/1259","labels_url":"https://api.github.com/repos/UnionOfRAD/lithium/issues/1259/labels{/name}","comments_url":"https://api.github.com/repos/UnionOfRAD/lithium/issues/1259/comments","events_url":"https://api.github.com/repos/UnionOfRAD/lithium/issues/1259/events","html_url":"https://github.com/UnionOfRAD/lithium/issues/1259","id":126505253,"number":1259,"title":"String::compare is not constant-time","user":{"login":"nanobe","id":1503853,"avatar_url":"https://avatars.githubusercontent.com/u/1503853?v=3","gravatar_id":"","url":"https://api.github.com/users/nanobe","html_url":"https://github.com/nanobe","followers_url":"https://api.github.com/users/nanobe/followers","following_url":"https://api.github.com/users/nanobe/following{/other_user}","gists_url":"https://api.github.com/users/nanobe/gists{/gist_id}","starred_url":"https://api.github.com/users/nanobe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nanobe/subscriptions","organizations_url":"https://api.github.com/users/nanobe/orgs","repos_url":"https://api.github.com/users/nanobe/repos","events_url":"https://api.github.com/users/nanobe/events{/privacy}","received_events_url":"https://api.github.com/users/nanobe/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/UnionOfRAD/lithium/labels/bug","name":"bug","color":"e11d21"}],"state":"open","locked":false,"assignee":{"login":"davidpersson","id":29702,"avatar_url":"https://avatars.githubusercontent.com/u/29702?v=3","gravatar_id":"","url":"https://api.github.com/users/davidpersson","html_url":"https://github.com/davidpersson","followers_url":"https://api.github.com/users/davidpersson/followers","following_url":"https://api.github.com/users/davidpersson/following{/other_user}","gists_url":"https://api.github.com/users/davidpersson/gists{/gist_id}","starred_url":"https://api.github.com/users/davidpersson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/davidpersson/subscriptions","organizations_url":"https://api.github.com/users/davidpersson/orgs","repos_url":"https://api.github.com/users/davidpersson/repos","events_url":"https://api.github.com/users/davidpersson/events{/privacy}","received_events_url":"https://api.github.com/users/davidpersson/received_events","type":"User","site_admin":false},"milestone":null,"comments":6,"created_at":"2016-01-13T20:25:07Z","updated_at":"2016-01-13T21:50:37Z","closed_at":null,"body":"String::compare is documented as a constant-time string comparison function, but it actually leaks timing information in the character-by-character comparison loop.\r\n\r\nThe line \"$result = $result && ($known[$i] === $user[$i]);\" uses a logical \"and\", which means if the first comparison (\"$result\") is false, then the second comparison won't be evaluated. In other words, the loop is sped up after the first mismatched character, which means the overall string comparison completes sooner the earlier it finds a mismatched character. This is exactly what you don't want to have happen.\r\n\r\nSince logical \"and\" and \"or\" operators can short-circuit depending on the result of the first comparison, constant-time string comparison functions usually rely on bitwise operations instead. For example, you can compare two characters by doing a bitwise \"xor\" on their numeric byte values (resulting in 0 if they are equal) and accumulate any resulting errors into an integer result variable using a bitwise \"or\". If that result variable is still equal to 0 in the end, the strings are equal.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/UnionOfRAD/lithium/issues/comments/171448163","html_url":"https://github.com/UnionOfRAD/lithium/issues/1259#issuecomment-171448163","issue_url":"https://api.github.com/repos/UnionOfRAD/lithium/issues/1259","id":171448163,"user":{"login":"alexbowers","id":842974,"avatar_url":"https://avatars.githubusercontent.com/u/842974?v=3","gravatar_id":"","url":"https://api.github.com/users/alexbowers","html_url":"https://github.com/alexbowers","followers_url":"https://api.github.com/users/alexbowers/followers","following_url":"https://api.github.com/users/alexbowers/following{/other_user}","gists_url":"https://api.github.com/users/alexbowers/gists{/gist_id}","starred_url":"https://api.github.com/users/alexbowers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexbowers/subscriptions","organizations_url":"https://api.github.com/users/alexbowers/orgs","repos_url":"https://api.github.com/users/alexbowers/repos","events_url":"https://api.github.com/users/alexbowers/events{/privacy}","received_events_url":"https://api.github.com/users/alexbowers/received_events","type":"User","site_admin":false},"created_at":"2016-01-13T21:50:37Z","updated_at":"2016-01-13T21:50:37Z","body":"For example, i'm going to use the characters: 'Ð' and 'Â'\r\n\r\nord() returns them both as int(195)\r\n\r\nstrlen returns them both as having length of 2.\r\n\r\nDoing a foreach over the length on that returns null for the character since there is no character at the position (it has two character positions).\r\n\r\nFixing the function to report the string lengths properly (mb_strlen instead of strlen), shows the true bug here though: ord considers those characters to be the same:\r\n\r\nhttps://3v4l.org/vnvpc\r\n\r\nThe original returned false, so looks like it works, but i do not think it is safe, since it doesn't compare the strings properly."}}