search

rlaager / docsis

This program encodes a DOCSIS binary configuration file from a human-readable text configuration file.
http://docsis.sourceforge.net
GNU General Public License v2.0
115 stars 70 forks source link

Bug concerning dialplan in docsis #22

Closed zuxy closed 9 years ago

zuxy commented 9 years ago

The following lines inside docsis.c:add_dialplan() looks problematic:

local_char = 0x16 + fileSize; if (local_char < 0x80) { memcpy(tlvbuf + tlvbuflen, p_local_char, sizeof(local_char)); tlvbuflen += sizeof(local_char); } else { ...

local_char is of type char, and the highlighted lines might overflow, and the resulting config file can't be recognized by PC2.0 eDVA.

Introducing an intermediate variable of type int can cure the problem:

local_char = local_char_w = 0x16 + fileSize; if (local_char_w < 0x80) { memcpy(tlvbuf + tlvbuflen, p_local_char, sizeof(local_char)); tlvbuflen += sizeof(local_char); } else { ...

AdrianSimionov commented 9 years ago

Thank you very much for the interest in this project.

Is it possible to either create a code push or provide a config file which will reproduce the issue? What would be good would be the MTA config (binary and text) file and the dialplan text file.

Regards, Adrian.

rlaager commented 9 years ago

The proposed change (assuming local_char_w is typed as an unsigned int) does not protect against integer overflow in the general case (which is insanely annoying to deal with in C), but does seem like a decent real-world fix.

That said, I have absolutely no idea what this code is trying to accomplish, as I'm not familiar with the spec for the output in this scenario.

zuxy commented 9 years ago

This piece of code tries to determine whether to use the short form or the long form for length encoding as per ASN.1 BER (https://en.wikipedia.org/wiki/X.690#BER_encoding). If the length is shorter than or equal to 127 it can encoded in the short form (one byte representing the length), otherwise it should use the long form (0x80+n in the 1st byte, n following bytes representing the length; in this case, n=2).

AdrianSimionov commented 9 years ago

Can you please let me know if commit c4af4308bb1c78101c99b2b8a9b231da5f72b9d4 is fixing the issue and eDVA recognizes the config file?

I consider two bytes for the size of the dialplan should be enough as per ITU-T J.460.4 (06/2008) pktcEUERSTDMValue is encoded as OCTET STRING(SIZE(0..8192)

I cannot test this commit because I do not have access at this moment to a real world setup.

AdrianSimionov commented 9 years ago

Fixed by commit c4af4308bb1c78101c99b2b8a9b231da5f72b9d4.