{"action":"created","issue":{"url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus/issues/30","repository_url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus","labels_url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus/issues/30/labels{/name}","comments_url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus/issues/30/comments","events_url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus/issues/30/events","html_url":"https://github.com/coassoftwaresystems/delphi-modbus/issues/30","id":337201712,"node_id":"MDU6SXNzdWUzMzcyMDE3MTI=","number":30,"title":"stack overflow in TIdModBusClient.SendCommand","user":{"login":"petergmorgan","id":33248282,"node_id":"MDQ6VXNlcjMzMjQ4Mjgy","avatar_url":"https://avatars1.githubusercontent.com/u/33248282?v=4","gravatar_id":"","url":"https://api.github.com/users/petergmorgan","html_url":"https://github.com/petergmorgan","followers_url":"https://api.github.com/users/petergmorgan/followers","following_url":"https://api.github.com/users/petergmorgan/following{/other_user}","gists_url":"https://api.github.com/users/petergmorgan/gists{/gist_id}","starred_url":"https://api.github.com/users/petergmorgan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petergmorgan/subscriptions","organizations_url":"https://api.github.com/users/petergmorgan/orgs","repos_url":"https://api.github.com/users/petergmorgan/repos","events_url":"https://api.github.com/users/petergmorgan/events{/privacy}","received_events_url":"https://api.github.com/users/petergmorgan/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-06-30T09:35:28Z","updated_at":"2018-07-02T09:29:53Z","closed_at":null,"author_association":"NONE","body":"Hi, \r\nWe ran into a issue (using Indy10, Delphi 2007) where our application was crashing to desktop and discovered there can be an overflow in \r\nIdModbusClient\r\n`Move(RecBuffer[0], ReceiveBuffer, iSize)`\r\nif the server responds with too much data on the input buffer since MBPData is a fixed size but the client doesn't check if the amount of data returned will fit into the recievebuffer.\r\n\r\nFor my purposes I've added a simple size check and thrown away the rest of the buffer in that cycle and that's stopped the overflow (and random CTD) here."},"comment":{"url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus/issues/comments/401732539","html_url":"https://github.com/coassoftwaresystems/delphi-modbus/issues/30#issuecomment-401732539","issue_url":"https://api.github.com/repos/coassoftwaresystems/delphi-modbus/issues/30","id":401732539,"node_id":"MDEyOklzc3VlQ29tbWVudDQwMTczMjUzOQ==","user":{"login":"JensMertelmeyer","id":1827784,"node_id":"MDQ6VXNlcjE4Mjc3ODQ=","avatar_url":"https://avatars3.githubusercontent.com/u/1827784?v=4","gravatar_id":"","url":"https://api.github.com/users/JensMertelmeyer","html_url":"https://github.com/JensMertelmeyer","followers_url":"https://api.github.com/users/JensMertelmeyer/followers","following_url":"https://api.github.com/users/JensMertelmeyer/following{/other_user}","gists_url":"https://api.github.com/users/JensMertelmeyer/gists{/gist_id}","starred_url":"https://api.github.com/users/JensMertelmeyer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JensMertelmeyer/subscriptions","organizations_url":"https://api.github.com/users/JensMertelmeyer/orgs","repos_url":"https://api.github.com/users/JensMertelmeyer/repos","events_url":"https://api.github.com/users/JensMertelmeyer/events{/privacy}","received_events_url":"https://api.github.com/users/JensMertelmeyer/received_events","type":"User","site_admin":false},"created_at":"2018-07-02T09:29:53Z","updated_at":"2018-07-02T09:29:53Z","author_association":"CONTRIBUTOR","body":"Yes, using `Move(..)` without any checks is extremely dangerous. A crash to desktop is plausible, since `Move(..)` does not do any checks at all. I believe replacing this line with\r\n`ReceiveBuffer := TBitConverter.InTo<TModBusResponseBuffer>( TBytes(RecBuffer) );`\r\nwith no other changes at all does solve the memory corruption issue.\r\n\r\nThe current behaviour is going to be the same when the reply is too long and raise an range check error (`ERangeError`) when the received reply is not sufficient to fill a `TModbusResponseBuffer`.\r\n\r\nPlease let me know if I should submit a pull request.\r\n"}}
{"actor":{"display_login":"JensMertelmeyer"}}