-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add a check for eof to avoid adding empty string to the socket data. #27
base: master
Are you sure you want to change the base?
Add a check for eof to avoid adding empty string to the socket data. #27
Conversation
Hi - just to say I forked the repo and applied this fix and it works great. |
This PR introduces support for DSN connection strings to simplify the configuration process. Users can now use a single string to determine the type of connection (`Network` or `Pipe`) and its details. This PR resolves appwrite#28, closes appwrite#25, resolves appwrite#26, closes appwrite#27, closes appwrite#31 ### Changes: 1. Added a static `createFromDSN` method to the `ClamAV` abstract class. This method parses the provided DSN string and returns an appropriate instance (`Network` or `Pipe`). 2. Updated the `ClamAVTest` unit test to cover the new DSN creation functionality. 3. Updated composer dependencies 4. Applied smaller code, and documentation optimizations 5. Updated README ### Usage: Users can now initialize a connection using a DSN string: ```php $clam = ClamAV::createFromDSN('tcp://localhost:3310'); $version = $clam->version(); ``` This approach provides a more flexible and user-friendly way to set up a connection. ### Testing: The existing unit tests have been updated accordingly.
This PR introduces support for DSN connection strings to simplify the configuration process. Users can now use a single string to determine the type of connection (`Network` or `Pipe`) and its details. This PR resolves appwrite#28, closes appwrite#25, resolves appwrite#26, closes appwrite#27, closes appwrite#31 1. Added a static `createFromDSN` method to the `ClamAV` abstract class. This method parses the provided DSN string and returns an appropriate instance (`Network` or `Pipe`). 2. Updated the `ClamAVTest` unit test to cover the new DSN creation functionality. 3. Updated composer dependencies 4. Applied smaller code, and documentation optimizations 5. Improved code coverage 6. Updated README Users can now initialize a connection using a DSN string: ```php $clam = ClamAV::createFromDSN('tcp://localhost:3310'); $version = $clam->version(); ``` This approach provides a more flexible and user-friendly way to set up a connection. The existing unit tests have been updated accordingly.
This PR introduces support for DSN connection strings to simplify the configuration process. Users can now use a single string to determine the type of connection (`Network` or `Pipe`) and its details. This PR resolves appwrite#28, closes appwrite#25, resolves appwrite#26, closes appwrite#27, closes appwrite#31 1. Added a static `createFromDSN` method to the `ClamAV` abstract class. This method parses the provided DSN string and returns an appropriate instance (`Network` or `Pipe`). 2. Updated the `ClamAVTest` unit test to cover the new DSN creation functionality. 3. Updated composer dependencies 4. Applied smaller code, and documentation optimizations 5. Improved code coverage 6. Updated README Users can now initialize a connection using a DSN string: ```php $clam = ClamAV::createFromDSN('tcp://localhost:3310'); $version = $clam->version(); ``` This approach provides a more flexible and user-friendly way to set up a connection. The existing unit tests have been updated accordingly.
This fix still fails with file https://github.com/fire1ce/eicar-standard-antivirus-test-files/blob/master/eicar-adobe-acrobat-javascript-alert.pdf |
@rang501 will removing the chunking cause issues for large files though? |
Not sure, we have file size limits (under 100MB), so this should not be a problem. I checked how the Drupal module does this and I didn't see any chunking, it was fully sent to Clamav. Anyway, the current fix did improve detection but did not fix it fully. I see it as a security hole to bypass detection and made a decision to skip chunking which worked and the file was detected. The failed files were https://github.com/fire1ce/eicar-standard-antivirus-test-files: |
Drupal source code here, for anyone working on this project |
Non-chunking version of the method is below. I don't think the chunking is necessary actually because stream_copy_to_stream does this internally anyway. public function fileScanInStream(string $file): bool
{
$file_handler = fopen($file, 'r');
$scanner_handler = socket_export_stream($this->getSocket());
// Push to the ClamAV socket.
$bytes = filesize($file);
fwrite($scanner_handler, "zINSTREAM\0");
fwrite($scanner_handler, pack("N", $bytes));
stream_copy_to_stream($file_handler, $scanner_handler);
// Send a zero-length block to indicate that we're done sending file data.
fwrite($scanner_handler, pack("N", 0));
// Request a response from the service.
$response = trim(fgets($scanner_handler));
fclose($scanner_handler);
return preg_match('/^stream: OK$/', $response);
}` |
Also using the version from #27 (comment) - works like a charm. Would be great to see this merged. |
This PR works for me. Please merge. |
What does this PR do?
Do not send the last empty string through the socket for files of less than 8192 bytes.
This avoid detection errors for eicar files.
Test Plan
Tested the function with multiple eicar files.
Related PRs and Issues
Resolve : #26
Have you read the Contributing Guidelines on issues?
(Write your answer here.)