Hi, all!

Binkd on Windows 10 32-bit. Is there a way to block a particular node from doing a file request? I've one node who keeps requesting a packet (that doesn't exist) and that causes Binkd to crash! :-(

+ 11 Nov 15:20:41 [1980] pwd protected session (MD5)
+ 11 Nov 15:20:41 [1980] sending c:\OUTBOUND\000f1d91.th0 as 000f1d91.th0 (165816)
- 11 Nov 15:20:41 [1980] receiving 61e1ed3d.pkt (186 byte(s), off 0)
? 11 Nov 15:20:41 [1980] GET: remote requests seeking 000f1d91.th0 to 180224, file size I64u

Appreciate any help!



Cheers... Dallas

--- timEd/386 1.10.y2k+
* Origin: The BandMaster, Vancouver, CANADA (1:153/7715)

On 2021-11-11 16:19:30, Dallas Hinton (1:153/7715) wrote to all:

Binkd on Windows 10 32-bit. Is there a way to block a particular node
from doing a file request? I've one node who keeps requesting a packet (that doesn't exist) and that causes Binkd to crash! :-(

+ 11 Nov 15:20:41 [1980] pwd protected session (MD5)
+ 11 Nov 15:20:41 [1980] sending c:\OUTBOUND\000f1d91.th0 as
000f1d91.th0 (165816) - 11 Nov 15:20:41 [1980] receiving 61e1ed3d.pkt
(186 byte(s), off 0) ? 11 Nov 15:20:41 [1980] GET: remote requests
seeking 000f1d91.th0 to 180224, file size I64u

I suspect turning off logging temporarily will prevent the crash.

...

I don't have time to send a pull request, but for other developers reading this:

"I64u" is a printf-formatting bug. It should be "%I64u", which gets converted to the file size at runtime.

diff for protocol.c:

- Log (1, "GET: remote requests seeking %s to %" PRIuMAX ", file size " PRIuMAX,
+ Log (1, "GET: remote requests seeking %s to %" PRIuMAX ", file size %" PRIuMAX,
argv[0], (uintmax_t) offset, (uintmax_t) state->out.size);
- msg_sendf(state, M_ERR, "Invalid M_GET violates binkp: offset " PRIuMAX " after end of file, file %s size " PRIuMAX,
+ msg_sendf(state, M_ERR, "Invalid M_GET violates binkp: offset %" PRIuMAX " after end of file, file %s size %" PRIuMAX,

Though this isn't the only place in the binkd code where this bug exists.

Obviously checking whether PRIuMAX is prefixed with "%" in every call to Log() or msg_sendf() etc is visually difficult and error-prone.

This should instead be fixed at the source, in sys.h:

- #define PRIdMAX "I64i"
- #define PRIuMAX "I64u"
+ #define PRIdMAX "%I64i"
+ #define PRIuMAX "%I64u"

Then change the .c files that reference these macros accordingly.

--- GoldED+/BSD 1.1.5-b20180707
* Origin: Blizzard of Ozz, Melbourne, Victoria, Australia (3:633/267)

Hi, andrew -- on Nov 12 2021 at 15:37, you wrote:

seeking 000f1d91.th0 to 180224, file size I64u

I suspect turning off logging temporarily will prevent the crash.

I would never have thought of that -- many thanks! Would you suggest turning off console logging, disk logging, or both?


Cheers... Dallas

--- timEd/386 1.10.y2k+
* Origin: The BandMaster, Vancouver, CANADA (1:153/7715)

This should instead be fixed at the source, in sys.h:

Nice catch. After scanning through the source, it seems to be less search and replace with your first suggestion than the second (the sys.h one)?

I found the macro in the protocol.c as you pointed out, and then in ftnq.c and inbound.c, however here it's used properly. Any other places?

In the EXE file I64u occurs five times without the %. I64i is never used anywhere (except in a comment in snprintf.c). I'm not using the latest version of binkd although, so YMMV...



..

--- Mozilla/5.0 (Windows; U; Windows NT 5.1; sv-SE; rv:1.9.1.16) Gecko/20101125
* Origin: news://eljaco.se (2:203/2)

Hi andrew!

12 Nov 21, andrew clarke ==> Dallas Hinton:

Binkd on Windows 10 32-bit. Is there a way to block a particular node
from doing a file request? I've one node who keeps requesting a packet
(that doesn't exist) and that causes Binkd to crash! :-(

+ 11 Nov 15:20:41 [1980] pwd protected session (MD5)
+ 11 Nov 15:20:41 [1980] sending c:\OUTBOUND\000f1d91.th0 as
000f1d91.th0 (165816) - 11 Nov 15:20:41 [1980] receiving 61e1ed3d.pkt
(186 byte(s), off 0) ? 11 Nov 15:20:41 [1980] GET: remote requests
seeking 000f1d91.th0 to 180224, file size I64u

I suspect turning off logging temporarily will prevent the crash.

...

I don't have time to send a pull request, but for other developers reading this:

"I64u" is a printf-formatting bug. It should be "%I64u", which gets converted to the file size at runtime.

diff for protocol.c:

- Log (1, "GET: remote requests seeking %s to %" PRIuMAX ", file size " PRIuMAX,
+ Log (1, "GET: remote requests seeking %s to %" PRIuMAX ", file size %" PRIuMAX,
argv[0], (uintmax_t) offset, (uintmax_t) state->out.size);
- msg_sendf(state, M_ERR, "Invalid M_GET violates binkp: offset " PRIuMAX " after end of file, file %s size " PRIuMAX,
+ msg_sendf(state, M_ERR, "Invalid M_GET violates binkp: offset %" PRIuMAX " after end of file, file %s size %" PRIuMAX,

Many thanks! The patch was applied.

Though this isn't the only place in the binkd code where this bug exists.

Obviously checking whether PRIuMAX is prefixed with "%" in every call to Log() or msg_sendf() etc is visually difficult and error-prone.

This should instead be fixed at the source, in sys.h:

- #define PRIdMAX "I64i"
- #define PRIuMAX "I64u"
+ #define PRIdMAX "%I64i"
+ #define PRIuMAX "%I64u"

Then change the .c files that reference these macros accordingly.

This way was choosen due to use of the format "%8" PRIuMAX in ftnq.c.
I'm not sure is it better to create another macro PRIuMAX8 or keep existing PRIdMAX without '%'.

Lucky carrier,
Pavel
aka gul@gul.kiev.ua
--- GoldED+/LNX 1.1.5-b20160827
* Origin: II:CDLXIII/LXVIII (2:463/68)

On 2021-11-14 22:16:48, Pavel Gulchouck (2:463/68) wrote to andrew clarke:

- #define PRIdMAX "I64i"
- #define PRIuMAX "I64u"
+ #define PRIdMAX "%I64i"
+ #define PRIuMAX "%I64u"

Then change the .c files that reference these macros accordingly.

This way was choosen due to use of the format "%8" PRIuMAX in ftnq.c.
I'm not sure is it better to create another macro PRIuMAX8 or keep existing PRIdMAX without '%'.

My initial reaction was to create a PRIuMAX8. However, inttypes.h in C99 already defines PRIuMAX without the leading %.

And in hindsight I see PRIuMAX is only defined in binkd's sys.h only for pre-C99 versions of MSVC.

So for compatibility with inttypes.h it's best to leave sys.h alone and just hunt for missing % characters in the *.c files (unfortunately).

Normally modern compilers (gcc/clang) are good at flagging printf() format bugs as they're fairly common, so it's strange they are not doing it here, despite building with $(CC) -Wall.

--- GoldED+/BSD 1.1.5-b20180707
* Origin: Blizzard of Ozz, Melbourne, Victoria, Australia (3:633/267)

Hi, Dallas -- on Nov 11 2021 at 22:11, you wrote:

I would never have thought of that -- many thanks! Would you
suggest turning off console logging, disk logging, or both?

So I turned off all logging, and guess what? Binkd still crashes!!! Is there an option to prevent a node from even connecting?

Cheers... Dallas

--- timEd/386 1.10.y2k+
* Origin: The BandMaster, Vancouver, CANADA (1:153/7715)

Hi! Dallas,

On 11/25/2021 04:49 PM, you wrote to yourself:

I would never have thought of that -- many thanks! Would you
suggest turning off console logging, disk logging, or both?

So I turned off all logging, and guess what? Binkd still crashes!!! Is there an option to prevent a node from even connecting?

By using the old timer's skullduggery:

* change their session password & don't tell them; or,
* if there is none yet set, set a session password & don't tell them.

Either way, a binkP connection won't last long. Easy peasy. :)

Cheers,
Paul.

--- Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.4.0
* Origin: That is just f'ck'n beautiful', said Goldilocks. (3:640/1384.125)

Hello Dallas,

On Thursday November 25 2021 16:49, you wrote to you:

So I turned off all logging, and guess what? Binkd still crashes!!!

Do you by any chance start binkd with the '-C' option?

Mine does, so I do not use that option any more.


Cheers, Michiel

--- GoldED+/W32-MSVC 1.1.5-b20170303
* Origin: http://www.vlist.eu (2:280/5555)

Hi, Paul -- on Nov 26 2021 at 16:44, you wrote:

* change their session password & don't tell them; or,
* if there is none yet set, set a session password & don't tell
them.

<duh> I'll try that, thanks! I had only tried deleting the entry in linklist!



Cheers... Dallas

--- timEd/386 1.10.y2k+
* Origin: The BandMaster, Vancouver, CANADA (1:153/7715)

Hi, Michiel -- on Nov 26 2021 at 09:38, you wrote:

MvdV> Do you by any chance start binkd with the '-C' option?

No, I'm not. What is -C supposed to do?



Cheers... Dallas

--- timEd/386 1.10.y2k+
* Origin: The BandMaster, Vancouver, CANADA (1:153/7715)

Hello Dallas,

On Friday November 26 2021 07:17, you wrote to me:

MvdV>> Do you by any chance start binkd with the '-C' option?

No, I'm not. What is -C supposed to do?

Reload the config file when it changes.


Cheers, Michiel

--- GoldED+/W32-MSVC 1.1.5-b20170303
* Origin: http://www.vlist.eu (2:280/5555)

Hi, Michiel -- on Nov 26 2021 at 17:09, you wrote:

MvdV>> Do you by any chance start binkd with the '-C' option?

No, I'm not. What is -C supposed to do?

MvdV> Reload the config file when it changes.

Ah, thanks. I changed linklist.binkd while binkd was running and it noticed the change and reloaded ... all seems well!


Cheers... Dallas

--- timEd/386 1.10.y2k+
* Origin: The BandMaster, Vancouver, CANADA (1:153/7715)