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

Fix for https://github.com/HerculesWS/Hercules/issues/2349. #2583

Merged
merged 3 commits into from Nov 23, 2019
Merged

Fix for https://github.com/HerculesWS/Hercules/issues/2349. #2583

merged 3 commits into from Nov 23, 2019

Conversation

Kenpachi2k13
Copy link
Member

@Kenpachi2k13 Kenpachi2k13 commented Nov 22, 2019

Pull Request Prelude

Changes Proposed

ipban_log() only inserts the first 3 octets of an IP and an asterisk for last one, so we can set the columns length of list in ipbanlist table to 13.

Issues addressed: #2349

ipban_log() only inserts the first 3 octets of an IP and an asterisk for last one, so we can set the columns length to 13.


ALTER TABLE `ipbanlist` MODIFY `list` VARCHAR(13) NOT NULL DEFAULT '';
INSERT INTO `sql_updates` (`timestamp`) VALUES (1574463539);
Copy link
Member

@dastgirp dastgirp Nov 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add this in main.sql too (INSERT for sql_updates)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, sorry. Done!

ipban_log() only inserts the first 3 octets of an IP and an asterisk for last one, so we can set the columns length to 13.
@Kenpachi2k13 Kenpachi2k13 merged commit 7d5ce7b into HerculesWS:master Nov 23, 2019
@4144
Copy link
Contributor

4144 commented Nov 23, 2019

in this pr also useless merge commit

@MishimaHaruna MishimaHaruna added this to the Release v2019.12.15 milestone Dec 16, 2019
@pazkero
Copy link
Contributor

pazkero commented Feb 14, 2020

Question, some custom plugins which I've saw on the forums add a command called "@ipban", for example, which is a non-automatic way of adding IP Bans.

An IP official length is 15 or 16 (xxx.xxx.xxx.xxx). Wouldn't this cause issues when IP bans are not done automatically? (Also, in some rather specific cases, it might be a desired behavior to ban only the specific IP. Yes, I am aware of how dynamic IP works and why we usually use a star.)

TL;DR Doesn't this causes issues or SQL errors if there are IP Bans of length 15?

PS. Feel free to open an issue about it, if you think it relevant. I'm just curious.

@dastgirp
Copy link
Member

@pazkero
If you see the source, we always add only 3 octet and wildcard

ipban.c:

if (SQL_ERROR == SQL->Query(ipban->sql_handle, "INSERT INTO `%s`(`list`,`btime`,`rtime`,`reason`) VALUES ('%u.%u.%u.*', NOW() , NOW() +  INTERVAL %u MINUTE ,'Password error ban')",
			ipban->dbs->table, p[3], p[2], p[1], login->config->dynamic_pass_failure_ban_duration))

As you can see here, the value inserted is '%u.%u.%u.*'.

I don't know why this is done. Also for custom source, I guess you would need to manually change sql for now. I will open the issue on your behalf as you have mentioned it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants