search

OpenVPN / tap-windows6

Windows TAP driver (NDIS 6)
Other
785 stars 237 forks source link

Remove overly strict buffer length check #71

Closed bemasc closed 5 years ago

bemasc commented 5 years ago

This file contains a check that required both input and output buffers to have nonzero size when processing DeviceIoControl commands. This is too strict, since most commands require only input or output, not both. It's duplicative, because each command-handling block still has to check whether the relevant buffer is the right size. It violates the API definition, e.g.

lpInBuffer
...
This parameter can be NULL if dwIoControlCode specifies an operation that does not require input data.

Most importantly, it has caused problems for a number of developers: https://github.com/ambrop72/badvpn/issues/2 https://github.com/ambrop72/badvpn/issues/16

cron2 commented 5 years ago

Hi,

On Mon, Dec 17, 2018 at 12:46:11PM -0800, Benjamin M. Schwartz wrote:

-- Commit Summary --

  • Remove incorrect buffer length restriction

I'll have a look. We're working on a new release with a few bug fixes anyway, so that is a good time :-)

gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de

cron2 commented 5 years ago

ACK.

Patch indeed does what it says, all the individual switch/case blocks validate the length, so this check is superfluous and harmful. Merge!

Thanks

bemasc commented 5 years ago

Thanks for the quick review!