NuSkooler / enigma-bbs

ENiGMA½ BBS Software
https://nuskooler.github.io/enigma-bbs/
BSD 2-Clause "Simplified" License
548 stars 105 forks source link

Crash on exporting packet #104

Closed ghost closed 7 years ago

ghost commented 7 years ago

For :bug: bug reports, please fill out the information below plus any additional relevant information. If this is a feature request, feel free to clear the form.

Short problem description

Crashing when trying to export packet

buffer.js:1027
    throw new TypeError('"value" argument is out of bounds');
    ^

TypeError: "value" argument is out of bounds
    at checkInt (buffer.js:1027:11)
    at Buffer.writeUInt16LE (buffer.js:1085:5)
    at Packet.writePacketHeader (/home/pi/enigma-bbs/core/ftn_mail_packet.js:337:10)
    at Packet.writeHeader (/home/pi/enigma-bbs/core/ftn_mail_packet.js:921:14)
    at createNewPacket (/home/pi/enigma-bbs/core/scanner_tossers/ftn_bso.js:583:32)
    at /home/pi/enigma-bbs/node_modules/async/dist/async.js:3686:9
    at replenish (/home/pi/enigma-bbs/node_modules/async/dist/async.js:881:17)
    at iterateeCallback (/home/pi/enigma-bbs/node_modules/async/dist/async.js:866:17)
    at /home/pi/enigma-bbs/node_modules/async/dist/async.js:843:16
    at /home/pi/enigma-bbs/node_modules/async/dist/async.js:3691:13

I'm not sure if it's a configuration error on my part, I used optutil to generate most of it, including the new .NA importer. I've tried nuking my messages.sqlite3.

Environment

Expected behavior

Should create a packet

Actual behavior

Crashes

Steps to reproduce

Write a message on a ftn message board.

My config.js :

{
  general: {
    boardName: "Exotica BBS"
  }
  messageConferences: {
    local: {
      name: "Local"
      desc: "Local Areas"
      sort: 1
      default: true
      areas: {
        general: {
          name: "General"
          desc: "General Chatter"
          sort: 1
          default: true
        }
      }
    }
    fsxnet: {
      name: "fsxNet"
      desc: "[F]un, [S]imple, e[X]perimental Network"
      sort:2
      areas: {
        fsx_gen: {
          name: "Chat, Testing + More.."
          desc: "Chat, Testing + More.."
        }
        fsx_mys: {
          name: "Mystic BBS Support/Dev"
          desc: "Mystic BBS Support/Dev"
        }
        fsx_cry: {
          name: "Cryptographics"
          desc: "Cryptographics"
        }
        fsx_bot: {
          name: "Automated roBOT Posts"
          desc: "Automated roBOT Posts"
        }
      }
    }
  }
  messageNetworks: {
    originLine: "Exotica - andrew.homeunix.org:2023"
    ftn: {
      networks: {
        fsxnet: {
          localAddress: "21:1/125.1"
        }
      }
      areas: {
        fsx_gen: {
          network: "fsxnet"
          tag: "FSX_GEN"
          uplinks: [
            "21:1/125"
          ]
        }
        fsx_mys: {
          network: "fsxnet"
          tag: "FSX_MYS"
          uplinks: [
            "21:1/125"
          ]
        }
        fsx_cry: {
          network: "fsxnet"
          tag: "FSX_CRY"
          uplinks: [
            "21:1/125"
          ]
        }
        fsx_bot: {
          network: "fsxnet"
          tag: "FSX_BOT"
          uplinks: [
            "21:1/125"
          ]
        }
      }
    }
  }
  scannerTossers: {
    ftn_bso: {
      schedule: {
        import: every 1 hours or @watch:/home/pi/ftn/in/toss!.now
        export: every 1 hours or @immediate
      }
      paths: {
        inbound: /home/pi/ftn/in
        outbound: /home/pi/ftn/out
        secInbound: /home/pi/ftn/in_secure
      }
      defaultZone: 21
      defaultNetwork: fsxnet

      nodes: {
          "21:1/125": {
            archiveType: ZIP
            encoding: utf8
          }
     }
    }
  }
  archivers: {
    zip: {
      compressCmd: "7za"
      decompressCmd: "7za"
    }
  }
  logging: {
    level: "info"
  }
}
NuSkooler commented 7 years ago

@apamment Thanks for the report. I quickly skimmed over your config & didn't see anything, but it appears the system is not getting your local "origin" address when creating packet files. I'll test out your config tonight and report back / fix any issues.

NuSkooler commented 7 years ago

Fixed with 3f873f58776963a3cccaa9027c1de0599937c9a9. The main issue was that you have a dot address set as localAddress (21:1/125.1). This should be set as 21:1/125.

This did however expose a bug in the packet writer, which has now been corrected.

ghost commented 7 years ago

My enigma set up is a point though?

ghost commented 7 years ago

Ok, so I tried to set localAddress to 21:1/125 but when I sent the packet, crashmail interpreted as coming from 21:1/125.0, also said it was a bad packet:

Tossing 3b809eec.pkt (0K) from 21:1/125.0 (22-Mar-17 14:47:36) / 4d 21:1/125.0 doesn't receive FSX_GEN Packet header too short for msg #2 (offset 392) Renaming /home/andrew/ftn/inb/3b809eec.pkt to .bad

ghost commented 7 years ago

After some investigating, I think it's because crashmail only does type 2 packets whereas enigma does type 2+ packets?

NuSkooler commented 7 years ago

After some investigating, I think it's because crashmail only does type 2 packets whereas enigma does type 2+ packets?

You can set packetType to 2 in the node config.

See https://github.com/NuSkooler/enigma-bbs/blob/master/docs/msg_networks.md

ghost commented 7 years ago

It doesn't seem to make a difference. Maybe it's just an issue with crashmail. I don't understand fidonet packets enough. When I set my local address to the point number, crashmail spits out that it received a mail from 21:255/125.1 which I think is because of the section that says "see FSC-48" where it puts -1 in the net and the right net into the auxNet field. I'm not sure it's supposed to do that on ordinary type 2 packets? only 2+? I don't know. Either way, the header is too big it would seem as it's trying to get the header for message two (which is too short, as there is really only one message packet header).

Anyway, that's just my thinking, maybe I will see if paul can allocate me a new node for enigma.

NuSkooler commented 7 years ago

I'll go over packet 2 types in the code again tomorrow and double check everything. The fix I made simply writes a -1 (0xff since it's unsigned) in the field.

Are you sure CrashMail can't use 2+? Most systems handle them.

On Feb 21, 2017 10:40 PM, "Andrew Pamment" notifications@github.com wrote:

It doesn't seem to make a difference. Maybe it's just an issue with crashmail. I don't understand fidonet packets enough. When I set my local address to the point number, crashmail spits out that it received a mail from 21:255/125.1 which I think is because of the section that says "see FSC-48" where it puts -1 in the net and the right net into the auxNet field. I'm not sure it's supposed to do that on ordinary type 2 packets? only 2+? I don't know. Either way, the header is too big it would seem as it's trying to get the header for message two (which is too short, as there is really only one message packet header).

Anyway, that's just my thinking, maybe I will see if paul can allocate me a new node for enigma.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/NuSkooler/enigma-bbs/issues/104#issuecomment-281574966, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcH25qdZAABzDZduyG-taHok4RS3A89ks5re8pcgaJpZM4MG6hM .

ghost commented 7 years ago

No I'm not 100% sure.

NuSkooler commented 7 years ago

I just looked at CrashMail's source and they do indeed support type 2+ (which is basically the standard & default on all boards).

I'd like to look into this more, but without knowing exactly what needs to happen, it's going to be hard :) If you wouldn't mind, can you generate a packet using a point address that CrashMail does treat properly and attach it here?

ghost commented 7 years ago

I'll have a look at getting this for you soon, I'll need to set up another test bbs to get a packet.

NuSkooler commented 7 years ago

@apamment I received your test packet, thanks! I just got home late so I probably won't spend too much tonight, but should have time to look into it tomorrow.

NuSkooler commented 7 years ago

Please try again after pulling. I went over everything & compared these documents about 2+ packets:

Basically:

if orig_is_a_point_addr then
  auxNet = origNet
  origNet = 0xffff // -1
end

Using Mystic, I set my local address to include a point and exported. It did not do this. Instead, it appears to set the point in two locations (which the first document above mentions). Perhaps the format never quite caught on and I must break the spec (or, perhaps I'm misinterpreting something here).

In any case, at some point I thinks some of the ENiGMA FTN packet code could use some love: The import/export logic would probably be best broken up into individual module/classes.

streaps commented 7 years ago

Perhaps the format never quite caught on

IIRC packet type 2+ was already the common format back in the 90s.

streaps commented 7 years ago

It looks that squish doesn't set origNet to -1

https://github.com/sdudley/maximus/blob/1cc413a28df645aeb7170ac7524a05abf501e6ab/squish/s_misc.c#L542

https://github.com/sdudley/maximus/blob/1cc413a28df645aeb7170ac7524a05abf501e6ab/squish/s_toss.c#L953

I wonder what "not mandated by official 2+ spec" means.

streaps commented 7 years ago

I'm not sure what packet type Mystic tries to write. I guess the intention was to write 2+ packets, but it's not valid 2+. It neither writes auxNet nor capValid. It could be interpreted as valid 2 packet though.

https://github.com/streaps/mysticbbs/blob/f5ff7e016964bd94b3e15fd7a4399253a4c84ab3/mystic/mutil_echocore.pas#L113

https://github.com/streaps/mysticbbs/blob/f5ff7e016964bd94b3e15fd7a4399253a4c84ab3/mystic/mutil_echoexport.pas#L339

NuSkooler commented 7 years ago

@streaps Thanks for the links - these are very helpful. I think what is happening is a lot of software is creating "2+" packets, but not to spec. Looking at source (e.g. SBBS @ https://github.com/protomouse/synchronet/blob/master/src/sbbs3/sbbsecho.c#L5038), some software does.

I'd much rather follow the spec (which I think is the case now for 2+ in ENiGMA) but if that causes too many issues, perhaps that's why some have chosen not to do so.

Any thoughts very welcome!

ghost commented 7 years ago

@NuSkooler I just tried again with enigma as a point, now the Net is '0' and the packet is too short.

What may possibly be happening, and it's something that was happening with the mystic 1.10 alpha code is that it wasn't filling in the capValid and capWord fields. Crashmail verifies these are correct and if they are, it treats it as a type 2+ packet, (and gets the net from the auxnet field) if they are incorrect it assumes it is a type two field.

from https://fidonet.ozzmosis.com/echomail.php/ftsc_public/ee5f26e62e9d7bb3.html capValid Byte-swapped copy of the least-significant 15 bits of capWord. That is to say that the most significant bit of capWord is masked off, and the resulting two bytes are swapped. In C: capValid = ((capWord & 0x7f00) >> 8) | ((capWord & 0xff) << 8));

If the capValid and capWord fields do not match, the packet SHOULD
NOT be processed as a Type 2+ packet.
streaps commented 7 years ago

Here is a list of the supported packet formats of various tossers:

https://gist.github.com/streaps/f7332c321b70e4931fe80a0e52087270

streaps commented 7 years ago

@apamment

This is a known bug in Mystic (GPL), but it should have been fixed in newer (but closed source) releases.

streaps commented 7 years ago

I think the recommendation from FSP-1040 to write FSC-0048 packets is harmful and I find this proposal misleading. I haven't found any tosser that cannot read FSC-0039 packets. Several tossers can read FSC-0048 packets and a few FSC-0045 (2.2) packets, but most tossers write FSC-0039 packets only. Synchronet, which writes FSC-0048 packets, seems to be the exception.

I think the most compatible way is to read FSC-0039 2+, FSC-0048 2+ and 2.2 packets and write by default FSC-0039 2+ packets. Writing 2.2 packets could be a configurable option.

NuSkooler commented 7 years ago

@streaps Thank you for your time into this. That table you've built is extremely helpful! After seeing that, I think you're right: recommending FSC-0048 seems dangerous.

I think the most compatible way is to read FSC-0039 2+, FSC-0048 2+ and 2.2 packets and write by default FSC-0039 2+ packets. Writing 2.2 packets could be a configurable option.

I agree. I can make the 2+ setting in ENiGMA produce FSC-0039 2+ style, and perhaps have an 2+48 option for those that really want to produce that type -- in addition to the other standards. 2+ will remain the default output, but just not in the FSC-0048 style.

(For now I'll simply change the 2+ to FSC-0039. I need to re-work some of this code later on anyway)

Does that sound good?

streaps commented 7 years ago

Sounds good.

I don't think we need FSC-0048 anymore, when the software from the 90s settled on FSC-0039. If it's not a package from a point, it's the same anyway. I don't know if the rare case of a point's tosser speaking to a node's FTS-0001 only tosser exists anywhere in the world.

I would be interested in 2.2 packets for full 5D support though.

Anyway, as long as ENIGMA½ can write FSC-0039 point packets, I'm happy :).

NuSkooler commented 7 years ago

I just pushed a very minor change to write FSC-0039 style. I'll be breaking out the code later on to write the various types. For now, this should cover 99% of the cases it seems.

NuSkooler commented 7 years ago

@apamment or @streaps Can one of you comment if this worked out? If so I'll close this issue. The re-write allowing different formats will come down the line.

ghost commented 7 years ago

sorry I haven't been able to test this yet, I'll try and set up a test scenario tomorrow.

streaps commented 7 years ago

I haven't had the time to install and configure Enigma. If someone can provide a sample PKT file from Enigma I can test it with some tossers that don't support FSC-0048.

ghost commented 7 years ago

@NuSkooler OK, so it's "sort of" working The messages are going through, but for some reason crashmail is still reporting:

Packet header too short for msg #2 (offset 566)

Going to see if I can figure this one out

ghost commented 7 years ago

NOTE: I'm not sure what will happen if multiple messages get in the same packet, not sure how to test that

OK, I'm not sure if it's a crashmail quirk but it expects one more null byte at the end of the packet than what enigma is putting.

diff --git a/core/ftn_mail_packet.js b/core/ftn_mail_packet.js
index 324f08e..e3692cf 100644
--- a/core/ftn_mail_packet.js
+++ b/core/ftn_mail_packet.js
@@ -726,9 +726,9 @@ function Packet(options) {

                let msgBodyEncoded;
                try {
-                       msgBodyEncoded = iconv.encode(msgBody + '\0', options.encoding);
+                       msgBodyEncoded = iconv.encode(msgBody + '\0' + '\0', options.encoding);
                } catch(e) {
-                       msgBodyEncoded = iconv.encode(msgBody + '\0', 'ascii');
+                       msgBodyEncoded = iconv.encode(msgBody + '\0' + '\0', 'ascii');
                }

                return Buffer.concat( [
NuSkooler commented 7 years ago

@apamment This needs to occur in the export area as the message body should only contain a single null terminator.

From FTS-0001:

A pseudo-message beginning with the word 0000H signifies the end of the packet.

I just pushed a fix for this. Hopefully this one does it for you 🍻

ghost commented 7 years ago

It's working perfectly now, thanks a lot !

NuSkooler commented 7 years ago

Great, thanks for you testing and patience!