Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win32 API misuse: ReadFile #1573

Closed
dcoutts opened this issue Feb 4, 2020 · 0 comments
Closed

Win32 API misuse: ReadFile #1573

dcoutts opened this issue Feb 4, 2020 · 0 comments
Assignees
Labels
audit daedalus When the wallet switches from cardano-sl to the new node R9B audit issues raised by R9B Windows
Milestone

Comments

@dcoutts
Copy link
Contributor

dcoutts commented Feb 4, 2020

ouroboros-network/Win32-network/cbits/Win32Async.c in HsAsyncRead calls ReadFile specifying both an lpNumberOfBytesRead parameter and an lpOverlapped parameter. Per documentation at https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile “Use NULL for this parameter [lpNumberOfBytesRead] if this is an asynchronous operation to avoid potentially erroneous results.” If synchronous operation is desired instead, the value assigned to that parameter (numBytesRead) should be returned to the caller instead of discarded and lpOverlapped should not be specified. If asynchronous operation is desired, only the lpOverlapped parameter should be set.

As reported by Root9B.

@dcoutts dcoutts added networking Windows audit daedalus When the wallet switches from cardano-sl to the new node labels Feb 4, 2020
iohk-bors bot added a commit that referenced this issue Feb 4, 2020
1576: Fix #1573: ReadFile: pass NULL pointer to lpNumberOfBytesRead r=coot a=coot

We only use 'ReadFile' in asynchronous way, and thus we should pass
a null pointer:
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile

Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
@iohk-bors iohk-bors bot closed this as completed in 1975f87 Feb 4, 2020
@coot coot added this to the S6 2020-02-13 milestone Feb 5, 2020
@vhulchenko-iohk vhulchenko-iohk added the R9B audit issues raised by R9B label Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit daedalus When the wallet switches from cardano-sl to the new node R9B audit issues raised by R9B Windows
Projects
None yet
Development

No branches or pull requests

3 participants