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

OID values should be unsigned int #24

Open
rjrivero opened this issue Jan 17, 2017 · 4 comments
Open

OID values should be unsigned int #24

rjrivero opened this issue Jan 17, 2017 · 4 comments

Comments

@rjrivero
Copy link

rjrivero commented Jan 17, 2017

I think there is a problem in the readOID function when a value in the OID is higher than 2^31. It is returning negative numbers. For example:

var ber = require("asn1").Ber;

// Buffer contains OID 1.3.6.1.4.1.14988.1.1.5.1.1.19.2887117176 
var data = new Buffer([6,18,43,6,1,4,1,245,12,1,1,5,1,1,19,138,224,215,210,120]);
var reader = new ber.Reader (data)
console.log(reader.readOID());
// prints "1.3.6.1.4.1.14988.1.1.5.1.1.19.-1407850120"

A possible fix would be casting each value to unsigned integer before storing. I.e replacing values.push(value) with values.push(value >>> 0)

@varadi
Copy link

varadi commented Jan 19, 2017

I agree with @rjrivero. I ran into the same problem with 32bit OIDs and values.push(value >>> 0) should solve it? Please also

  • add a release note that this implantation limits OIDs to 32bit (readOID and writeOID)
  • log an error if an OID exceeds 32 bit, i.e. if the sequence has more than 5 bytes, or has 5 bytes but the first byte has bits 1, 2 or 3 set (bit 0 is 1).

@stephenwvickers
Copy link

I have seen there is little progress on this project over the past few years. My project, the net-snmp project, depends on this project, so I have cloned it and migrated (i.e. copied) all the outstanding issues and pull requests.

I will be releasing a new version of my version of this project under the asn1-ber namespace on npmjs.org by the end of the week which should address this particular item (I've already pushed the current code to this namespace to reserve it).

I am now tracking all changes under my own copy at asn1-ber. Note that this is not a "fork" but a clone and fresh commit to a new repository. I have of course retained all copyrights and attributed back the original author.

There are quite a few projects dependant on this project, and I want to make sure there is momentum on the issues and feature requests reported.

@stephenwvickers
Copy link

This issue has been addressed in my version of this project which has now been published to npm:

https://www.npmjs.com/package/asn1-ber

@varadi
Copy link

varadi commented Jul 24, 2017

@stephenwvickers: cool. Thank you.

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 a pull request may close this issue.

3 participants